-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BUG: Simple Aperture Photometry plugin to update when Subset updates #1447
Conversation
@@ -249,6 +250,13 @@ def test_annulus_background(imviz_helper): | |||
phot_plugin.bg_annulus_width = 5 | |||
assert_allclose(phot_plugin.background_value, 4.894003242594493) | |||
|
|||
# Move the last created Subset (ellipse) and make sure background updates | |||
imviz_helper.app.session.edit_subset_mode.mode = ReplaceMode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devs, I am not familiar with the edit mode syntax, so please let me know if it is correct to test this way.
Would also be nice if you can tell me how to programmatically control which Subset to put in ReplaceMode.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To programmatically set the subset in replace mode, you can do something like:
imviz_helper.app.session.edit_subset_mode.edit_subset = [imviz_helper.app.data_collection.subset_groups[0]]
Codecov Report
@@ Coverage Diff @@
## main #1447 +/- ##
==========================================
- Coverage 85.16% 85.15% -0.01%
==========================================
Files 91 91
Lines 8359 8369 +10
==========================================
+ Hits 7119 7127 +8
- Misses 1240 1242 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that this fixes the two use-cases explained in the PR description.
Someday I'd like to see this type of logic supported directly in the subset dropdown component, but the implementation here is pretty lightweight as-is, so let's defer that until/unless another plugin needs something similar (basically plugin-callbacks when the underlying data of the selected object changes).
Description
This pull request is to ensure Simple Aperture Photometry plugin is aware of Subset that is updated after it is created, if relevant to calculations. Most things will not recalculate until user presses "Calculate" but the background is calculated "live" before the button is pressed. There are 3 dropdowns. The dataset dropdown already triggers all recalculations, so I don't think it is affected but this bug.
This patch fixes the following cases:
Fixes #1443
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.
trivial
label.CHANGES.rst
?