Imviz simple aperture photometry plugin (no photutils)

[pllim]

Description

This pull request is to add a plugin to perform simple aperture photometry in Imviz.

This supersedes #930 . This is a simpler implementation that only works on default viewer and does not rely on photutils.

TODO:

• Actually implement the plugin.
• Fix id logic so it increments if appending successfully to QTable.
• Add change log.
• Polish up POC notebook for demo.
• Use mode='center' for normal stats.
• Handle pixel area correctly in aperture sum unit. New user input field for pixel scale in arcsec squared to be converted to steradians to get rid of it in JWST data unit.
• New user input field for flux to counts conversion factor.
• Update PR screenshots.
• Write tests. ⭐ (Also make sure it works with circle, ellipse, and rectangle. See "Gaussian Smooth" tests for example on how to test plugin.) "devdeps" job failure is unrelated, see TST: Imviz test failures in devdeps job #949.
• Update Imviz user-facing doc. (Hold off for now until we have finalized the code and UI.) See https://jdaviz--938.org.readthedocs.build/en/938/imviz/plugins.html
• ???
• Profit!

(Screenshots a bit outdated but you get the idea.)

Pre-requisite for #531 .

🐱

Fixes #531

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 #938 (8f3441b) into main (816d031) will increase coverage by 0.96%.
The diff coverage is 95.93%.

@@            Coverage Diff             @@
##             main     #938      +/-   ##
==========================================
+ Coverage   69.09%   70.06%   +0.96%     
==========================================
  Files          69       71       +2     
  Lines        4935     5121     +186     
==========================================
+ Hits         3410     3588     +178     
- Misses       1525     1533       +8     

Impacted Files	Coverage Δ
...imviz/plugins/aper_phot_simple/aper_phot_simple.py	95.37% <95.37%> (ø)
jdaviz/configs/imviz/helper.py	98.31% <100.00%> (+0.01%)	⬆️
jdaviz/configs/imviz/plugins/__init__.py	100.00% <100.00%> (ø)
...configs/imviz/plugins/aper_phot_simple/__init__.py	100.00% <100.00%> (ø)
jdaviz/configs/imviz/plugins/parsers.py	100.00% <100.00%> (ø)

---

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 816d031...8f3441b. Read the comment docs.

[PatrickOgle]

Nice! One request--Please add the option to display the results in counts. MJy/sr is also not the correct unit. Need to multiply by the pixel area in sr to get the result in Jy.

[larrybradley]

Probably out of scope, but in principle one could define a mask (e.g., from a DQ array perhaps).

[pllim]

Yes, that is out of scope for now. We have hard enough time getting stuff out of various headers, let alone getting even more arrays from different extensions.

[PatrickOgle]

@pllim We discussed this and agreed there should be an input field for counts-to-flux conversion factor.

[pllim]

@larrybradley , just so I understand, you expect user to calculate this by hand? (Ops, I pinged the wrong person, sorry! Cami explained it during tag-up on 2021-10-19.)

https://github.com/spacetelescope/synphot_refactor/blob/905cd1669617fd631c46f8ca21efe3336dfdb8fe/synphot/units.py#L151

[pllim]

Need to multiply by the pixel area in sr to get the result in Jy

This might be a can of worms. The image is not always MJy/sr. What I can do is to display the result unit with an additional * pix (or whatever makes sense) in the unit to make it clear that user needs to manually apply pixel scale to get something meaningful. @PatrickOgle , @larrybradley , et al., what do you think?

[PatrickOgle]

@pllim @larrybradley How about a pixel scale input field (in arcsec)? This could be converted to sr/pix, so that flux = MJy/sr * (sr/pix).
I don't want the user to have to pull out the calculator to convert every output themselves. Instead the plugin will do the calculation with the appropriate inputs.

[pllim]

How about a pixel scale input field (in arcsec)?

I think I need input from @larrybradley here as photutils expert. aperture_photometry() currently does not attempt to multiply output unit by pixel area. Does that mean aperture_photometry() output unit is also inaccurate and needs to be fixed? Am I missing something here?

<QTable length=2>
  id  xcenter ycenter    aperture_sum
        pix     pix        MJy / sr
int64 float64 float64      float64
----- ------- ------- ------------------
    1    30.0    30.0 28.274333882308134
    2    40.0    40.0 28.274333882308134

Also, if we create yet another user input field for pixel scale, is it always applicable for any number of image units, telescope, detector, etc? If someone does not know or care and just want the aperture sum, how should this plugin behave?

[PatrickOgle]

@pllim Default conversion factor could be for 1x1 sq arcsec pixel.

[pllim]

@PatrickOgle , the value is from image header.

PIXAR_A2= 0.003972344883408201 / Nominal pixel area in arcsec^2

[pllim]

@javerbukh , I added some stuff here but I don't remember if you wanted me to start this as another PR or not...

[pllim]

This is now feature-complete and ready for review. @larrybradley @PatrickOgle et al., please double check the math. Thanks!

[PatrickOgle]

1. There appears to be a problem with the x center value for circular aperture photometry if I load the two images acs_47tuc_1,2 and do photometry on acs_47tuc_1. (y center looks correct.) The Subset 1 coordinates are (x=1992.08203125, y=1037.489013671875) according to imviz.get_interactive_regions(). The photometry plugin outputs (1991.7647705078125, 1037.489013671875). In addition, the mouseover pixel x value is way off: (1969.5, 1037.5).

If I only load one image (acs_47tuc_1), with a different regions, I get (1991.309814453125 pix, 1037.7984619140625 pix) compared to (x=1991.309814453125, y=1037.7984619140625) from get_interactive_regions, so they match. The mouseover x values are still way off.

2. Can't easily compare the (ra,dec) coordinates of the center, which are (correctly) in decimal degrees to the sexigesimal mouseover coordinate values (which should be given in decimal degrees as well as sexigesimal).

3. The npix output looks correct (137.9 compared to 137.927... expected value).

4. The aperture sum and statistics for a star look approximately correct compared to DS9, given that I did not use exactly the same manually-selected region:

Imviz, DS9
center=(6.06912025, -71.88098929), (6.0691062, -71.880986)
npix = 137.9, 138
aperture_sum = 4.6842e+04 electron, 46912.069
mean = 3.4131e+02 electron, 339.94253
stddev = 8.3282e+02 electron, 829.93051
median = 9.9231e+01 electron, 99.231346 --match within sig figs
min = 3.8004e+01 electron, 38.004108 --match within sig figs
max = 7.2309e+03 electron, 7230.8984 --match within sig figs

The header for this ACS image has BUNIT="ELECTRONS"

[PatrickOgle]

If I leave the Flux scaling box empty, I get the following error when I calculate:
"Aperture photometry failed: ValueError("could not convert string to float:")

[rosteen]

The code looks good to me, everything looks sensible and works (as far as I can tell, perhaps @PatrickOgle will provide more rigorous validation of the calculation results). Nice addition.

Edit: I also meant to say that I appreciate the detailed documentation!

[pllim]

Thanks for the commit, @rosteen ! I do not mind at all. 😸

[PatrickOgle]

The outputs of the photometry tool match the aperture photometry from DS9, to better than 10%. The Poisson uncertainty on counts also seems to be correct and properly incorporates the flux/counts conversion factor. More accurate science validation TBD in a separate PR.

[pllim]

Thanks for the reviews!