Translator for regions and photutils

[pllim]

Description

This pull request is to implement translators to convert certain regions shapes to photutils apertures, primarily to support new features for Simple Aperture Photometry plugin that Imviz uses (e.g., centroid and FWHM).

This PR also follows up on #1131 by changing plot color to gray (original request) and making the profile plot marker size a little smaller.

https://jdaviz--1138.org.readthedocs.build/en/1138/imviz/plugins.html#simple-aperture-photometry

References:

• https://astropy-regions.readthedocs.io/en/stable/shapes.html
• https://photutils.readthedocs.io/en/stable/aperture.html
• https://github.com/astropy/photutils/blob/main/photutils/aperture/stats.py#L51

Blocked by:

• BUG: Fix ellipse translation glue-viz/glue-astronomy#67
• MNT: Drop Python 3.7 support #1145
• ApertureStats not optimized for large array even when aperture is small astropy/photutils#1316

This is required for:

• Fix [FEAT] Imviz: add fwhm to the aperture photometry plugin #1048
• Fix Optimize aperture photometry for big data #1155

Related PRs/issues:

• BUG: Fix ApertureStats give nan centroid astropy/photutils#1311
• Add ApertureStats class astropy/photutils#1309
• DOC: Add ref to Aperture Photometry top level astropy/photutils#1299
• DOC: Clarify ellipse semimajor/minor axes astropy/regions#435
• Ellipse aperture sum inconsistent with photutils astropy/regions#442

TODO

• Add milestone after 2.3 release on Mar 2.
• Add change log, including a mention that photutils now a required dependency.
• Update photutils dependency in docs/dev/jdaviz.svg.
• Implement translators.
• Write new translator tests.
• Drop Python 3.7, see Translator for regions and photutils #1138 (comment) . This is now MNT: Drop Python 3.7 support #1145
• Make sure the translator only do copies of parameters, not by reference. (No need due to descriptor magicks.)
• Refactor Simple Aperture Photometry plugin to use the new translators, maybe.
• Update existing tests of affected code.
• Update code to use new ApertureStats class in photutils.
• See if we can also fix big mosaic performance issues here? Yes, fixed upstream in photutils.
• Pin regions to a stable release that is >0.5 in setup.cfg after that release happens. Now it pins to dev to make tests pass.
• Pin glue-astronomy to a stable release that is >=0.3.3.
• Pin photutils to a stable release with that feature branch. Also update interphinx URL.
• ???
• Profit!

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 change log needed? If yes, is it added to CHANGES.rst?
• Is a milestone set? Milestone is only currently required for PRs related to Imviz MVP.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)? 🐱 🐱 🐱 🐱

[codecov]

Codecov Report

Merging #1138 (dbb2bb7) into main (56fcebf) will increase coverage by 0.14%.
The diff coverage is 99.15%.

@@            Coverage Diff             @@
##             main    #1138      +/-   ##
==========================================
+ Coverage   77.29%   77.44%   +0.14%     
==========================================
  Files          89       90       +1     
  Lines        7028     7079      +51     
==========================================
+ Hits         5432     5482      +50     
- Misses       1596     1597       +1     

Impacted Files	Coverage Δ
jdaviz/core/region_translators.py	98.50% <98.50%> (ø)
...imviz/plugins/aper_phot_simple/aper_phot_simple.py	94.80% <100.00%> (-0.34%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56fcebf...dbb2bb7. Read the comment docs.

[rosteen]

Other than a vague sense that it would be useful for these translators to live somewhere other than Jdaviz, I think this looks good. The bug that I found is also present on main so it doesn't block this.

[pllim]

it would be useful for these translators to live somewhere other than Jdaviz

Yes. @larrybradley and I cannot agree between photutils and regions. But I think the end goal is they would move upstream some day.

[duytnguyendtn]

Ran through a simple aperture photometry example in Imviz and everything seems to be extracted as I would expect! Thanks for the increment!

[duytnguyendtn]

I will note I agree with @rosteen's comment:

it would be useful for these translators to live somewhere other than Jdaviz

Not sure if glue-astronomy would be the most appropriate place since this isn't exactly related to the glue stuff that's already in there?

[pllim]

No, no one really uses glue-astronomy but us. It would be wasted to get the translation in there. Some day, either Larry will convince me that it should go into regions or I will him that it should be in photutils... 😆

[pllim]

Thanks for the reviews!