Deprecate subset getters

[duytnguyendtn]

Description

This PR identifies existing locations in our codebase that utilize our old subset and data getters and replaces it with our new get_subsets/getdata methods. Specifically, the following three changes:

1. get_subsets_from_viewer() replaced with get_subsets()
2. get_data_from_viewer() replaced with get_data()
3. specviz.get_spectra(subset_to_apply) replaced with specviz.get_spectra(spectral_subset)

In addition, I had to fix a bug with the Mosviz2DProfileViewer.get_data() method where it was returning statistic=mean of the Spectrum1D object by default due to not having the ability to specify the statistic. I added a keyword argument and changed the default to statistic=None

I think I got all the docs changes, but let me know if I missed something!

FYI decreased coverage is due to the get_subsets_from_viewer and get_data_from_viewer methods not being covered by tests anymore

Fixes #2224

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[codecov]

Codecov Report

Patch coverage: 87.09% and project coverage change: -0.40 ⚠️

Comparison is base (6a79df3) 91.76% compared to head (06f2ed0) 91.37%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2242      +/-   ##
==========================================
- Coverage   91.76%   91.37%   -0.40%     
==========================================
  Files         150      150              
  Lines       16847    16862      +15     
==========================================
- Hits        15460    15407      -53     
- Misses       1387     1455      +68     

Impacted Files	Coverage Δ
jdaviz/core/template_mixin.py	92.86% <ø> (ø)
jdaviz/app.py	86.06% <33.33%> (-7.40%)	⬇️
jdaviz/core/helpers.py	87.17% <50.00%> (ø)
jdaviz/configs/specviz/helper.py	77.41% <80.00%> (+0.37%)	⬆️
...nfigs/cubeviz/plugins/tests/test_data_retrieval.py	100.00% <100.00%> (ø)
...aviz/configs/cubeviz/plugins/tests/test_parsers.py	97.95% <100.00%> (ø)
jdaviz/configs/cubeviz/plugins/tests/test_tools.py	100.00% <100.00%> (ø)
jdaviz/configs/imviz/tests/test_helper.py	100.00% <100.00%> (ø)
jdaviz/configs/mosviz/plugins/viewers.py	90.43% <100.00%> (+0.43%)	⬆️
jdaviz/configs/mosviz/tests/test_data_loading.py	100.00% <100.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[pllim]

Also, are you feeling adventurous? 😆

#2201 (review)

@javerbukh , "... do we want to keep using get_interactive_regions() over get_subsets()?"

[duytnguyendtn]

Also, are you feeling adventurous? 😆

#2201 (review)

@javerbukh , "... do we want to keep using get_interactive_regions() over get_subsets()?"

Falling a little behind schedule so I'm going to leave this work out

[javerbukh]

Not a blocker but when I do specviz.app.get_subsets_from_viewer("spectrum-viewer") I see

WARNING: AstropyDeprecationWarning: The get_subsets_from_viewer function is deprecated and may be removed in a future version.
        Use get_subsets instead. [warnings]
WARNING:astroquery:AstropyDeprecationWarning: The get_subsets_from_viewer function is deprecated and may be removed in a future version.
        Use get_subsets instead.
WARNING: AstropyDeprecationWarning: The get_data_from_viewer function is deprecated and may be removed in a future version.
        Use get_data instead. [jdaviz.app]
WARNING:astroquery:AstropyDeprecationWarning: The get_data_from_viewer function is deprecated and may be removed in a future version.
        Use get_data instead.

I understand why the warning was issued for get_data_from_viewer as well but why were both warnings printed twice?

[pllim]

astroquery:AstropyDeprecationWarning ??? Why is it calling astroquery?

[duytnguyendtn]

I'm not sure; the even weirder thing is that it's warning is coming from astroquery??

[duytnguyendtn]

app.py doesn't even import anything from astroquery!!

[pllim]

Try turning warning into error in your session and inspect the traceback. Maybe somewhere deep in the machinery, one deprecated function is calling another deprecated function.

[pllim]

Please add a change log. Thanks!

[pllim]

Also need a rebase because that one-liner bug fix has been merged as #2258 .

[pllim]

I applied my review as commits, so LGTM now. @javerbukh will do the final review.

[javerbukh]

Looks good, thank you!

[duytnguyendtn]

Thank you everyone for merging this in my absence!