photutils
photutils copied to clipboard
astropy
Add curve-of-growth-based FWHM estimator
TODO
- [ ] Name a Jdaviz dev to take over this PR.
- [ ] Compare this to what is already in imexam. Which one should we use?
- [ ] Finalize API.
- [ ] Implement chosen algorithm.
- [ ] Add, update, and fix tests.
- [ ] Add and update documentation.
- [ ] Update change log, if necessary.
Original post
This adds a function that basically implements the algorithm I suggested for a project that wants to use photutils: spacetelescope/jdaviz#1048 - I described the algorithm there, but it's basically "put down an aperture, consider "max" to be all of the flux, then find the radius that provides half of that flux to be the half-width-at-half-max.
This isn't the most robust algorithm in the world for FWHMs, but it is probably conceptually the simplest.
This is a working PR, but I should probably document it a bit better and do more comprehensive tests, so I'm opening it for now as a draft. @larrybradley in particular might want to comment if you think I've put this in the right sub-package...
cc @pllim @larrybradley
Hello @eteq! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:
- In the file
photutils/morphology/fwhm.py:
Line 14:33: E128 continuation line under-indented for visual indent Line 20:1: E303 too many blank lines (3) Line 98:101: E501 line too long (118 > 100 characters) Line 121:101: E501 line too long (138 > 100 characters) Line 142:13: E265 block comment should start with '# ' Line 142:101: E501 line too long (104 > 100 characters) Line 153:88: E262 inline comment should start with '# ' Line 153:101: E501 line too long (144 > 100 characters) Line 160:101: E501 line too long (101 > 100 characters) Line 161:66: E225 missing whitespace around operator Line 162:34: E127 continuation line over-indented for visual indent Line 182:101: E501 line too long (112 > 100 characters) Line 184:42: W292 no newline at end of file
- In the file
photutils/morphology/tests/test_fwhm.py:
Line 1:1: F401 'curses.COLOR_GREEN' imported but unused Line 13:1: E302 expected 2 blank lines, found 1 Line 20:32: E261 at least two spaces before inline comment Line 20:32: E262 inline comment should start with '# ' Line 21:26: E231 missing whitespace after ',' Line 21:28: E231 missing whitespace after ',' Line 21:30: E231 missing whitespace after ',' Line 26:20: E261 at least two spaces before inline comment
We need to compare this with what is already established in imexam, see https://github.com/spacetelescope/imexam/blob/b3975733e0ef459fc7d137acec9bf4aa2865d6c5/imexam/imexamine.py#L1266 . It also uses photutils for photometry.
I am guessing this is going to be a blocker for both spacetelescope/jdaviz#1048 and spacetelescope/jdaviz#1049 ?
Can we even use this without a way to translate between regions region and photutils region first?
@eteq , we discussed this at Indigo tag-up today. I modified your original post with our plan. Please look at it and let us know if you have questions or concerns.
We need to compare this with what is already established in imexam,
Honestly I am not convinced we should give the imexam implementation precedence, as the approach in this PR in some ways is intentionally different (e.g., how it treats fractional pixels). I agree a comparison does make sense for sanity-checking and to ensure I'm not making a mistake in something here, but the answer might still be "use this instead of imexam" even if they are different.
Rest of the plan looks good to me! Although even if someone else can/wants to take this over I still care a lot about the answer to this algorithm ion photutils regardless of how it gets used in jdaviz.
Oh, and I also think this is already in the right direction - the implementation and API are basically complete modulo any concrete changes. But I would be very happy to get some help making more comprensive tests, as then I could focus just on docs!