Ensure that existing manipulation and analysis functions use the `mask` where appropriate.
- Dominant language
- Python
- Stars
- 204
- Forks
- 134
- Avg merge
- 12h 55m
- Merged PRs (30d)
- 1
Description
This comes out of #509, which introduces a function that computes and assigns `mask`'s to `Spectrum1D` objects. This led to a discussion where it was realized that a lot of the downstream analysis tasks that would follow on from #509 need to respect the `mask` for #509 to actually be practically useful - without that it seemed like a `flux_masked` or similar seemed necessary, which is not desirable since it essentially replicates the mask in `Spectrum1D` itself.
So the point of this issue is to make sure the currently-existing `analysis` and `manipulation` tools are mask-aware. At the time of writing this, I think the current status is:
* `specutils.analysis` functions: `centroid`, `equivalent_width`, `fwhm`, `gaussian_fwhm`, `gaussian_sigma_width`, `line_flux`, `snr` - all of these ignore the mask entirely, but I think it's a pretty easy thing to make them work: if the mask is `None`, do what it does now, otherwise, just do `self.flux[~self.mask]` where `self.flux` currently appears (and `self.spectral_axis` -> `self.spectral_axis[~self.mask]`, although they don't all use the spectral axis).
* line and continuum fitting (same underlying implementation. in `fit_lines`): I think this is *also* basically a one-liner in the implementation, doing essentially the above, but in the call to the fitter.
* parameter estimators in @brechmos-stsci may be able to advise here, but I *think* these are all eventually implemented via the `analysis` functions, right? So then if we fix those then it automatically carries over to the estimators?
* `extract_region`: already does the correct thing of just carrying through the mask.
* arithmetic: this already does the right thing by acarrying the mask thorough, I think by virtue of leaning on the existing `NDData` machinery.
The one question in my mind that applies to all of the above: do we want `use_mask` to be a keyword we add to *all* of these? My initial inclination is no: we always use the mask, but if the user does not want that, they can just set the mask to None. Or alternatively, we add a `without_mask` method to `Spectrum1D` that just copies the spectrum but sets the mask to None. Then the user could just do things like `function(spectrum.without_mask()` instead of `function(spectrum, use_mask=False`. Any preferences from others? If not I think I like the `without_mask` option.
cc @brechmos-stsci @camipacifici @nmearl
Contributor guide
Research direction
Start with the listed specutils.analysis functions and the shared fit_lines implementation, then compare their current handling with extract_region and arithmetic, which already carry masks through. Determine how the mask should affect each analysis and fitting path, and resolve whether mask use is always-on or configurable. Done means the existing analysis and fitting tools consistently respect Spectrum1D masks.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- data
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100