Fix aperture photometry bug for cubes in deconfigged#3897
Conversation
e554b9e to
d1b25bf
Compare
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (89.28%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3897 +/- ##
==========================================
+ Coverage 88.20% 88.28% +0.07%
==========================================
Files 198 198
Lines 26904 26914 +10
==========================================
+ Hits 23732 23761 +29
+ Misses 3172 3153 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # convert pixarea, which is in arcsec2/pix2 to the display solid angle unit / pix2 | ||
|
|
||
| if self.config in ('imviz', 'deconfigged'): | ||
| if self.config == 'imviz': |
There was a problem hiding this comment.
should this also be for images (not cubes) in deconfigged or no (because deconfigged has unit-conversion and imviz does not)?
There was a problem hiding this comment.
Oo yeah good catch, do you think a dim check is enough?
a8b5e58 to
261082a
Compare
| assert plg._obj.cube_slice == "4.894e+00 um" | ||
| plg = helper.plugins["Aperture Photometry"] | ||
| assert plg.dataset.labels == [flux_label, unc_label] | ||
| assert_quantity_allclose(plg._obj.cube_slice, 4.89 * u.um, atol=1e-2 * u.um) |
There was a problem hiding this comment.
this tolerance seems pretty low, did something change so doesn't match as well anymore?
There was a problem hiding this comment.
At some point in my testing, deconfigged was returning a slightly different value than cubeviz (on the order of 1e-3 I think) so I added the allclose. But I just tested again with the old exact value and it works so I'll revert that!
2df976c to
f7072e6
Compare
* Minor adjustment to dim check for units.
575b9bb to
a354077
Compare
Description
This pull request is to address an error that occurs when aperture photometry is attempted on cubes in deconfigged. The issue was that several of the checks in the code were hardcoded to
imvizandcubevizbut were not updated with the introduction of cube loaders. Those checks were updated throughout the code (mostly to dimensionality checks).Change log entry
CHANGES.rst? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rstbefore merge. If no, maintainershould add a
no-changelog-entry-neededlabel.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.
triviallabel.cache-download.ymlworkflow?