-
Notifications
You must be signed in to change notification settings - Fork 76
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
Extract on SubsetCreateMessage #3238
base: main
Are you sure you want to change the base?
Conversation
@@ -262,6 +262,7 @@ def _aperture_items_changed(self, msg): | |||
self._extract_in_new_instance(subset_lbl=subset_lbl, | |||
auto_update=True, add_data=True) | |||
except Exception: | |||
raise |
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.
This is here for debugging to expose the error.
self.hub.subscribe(self, SubsetCreateMessage, | ||
handler=lambda msg: self._update_subset(msg.subset)) |
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.
I hoped that I could just replace the trigger for this from SubsetUpdateMessage
to SubsetCreateMessage
, but it didn't work.
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.
Probably cannot work as a drop-in replacement since SubsetCreateMessage
does not have an attribute
(in fact, does nothing). Wondering if this would need something equivalent of LayerSelect._on_subset_created
, or if _update_subset
can tricked into doing the right action some other way.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3238 +/- ##
==========================================
- Coverage 87.62% 87.60% -0.02%
==========================================
Files 128 128
Lines 19844 19857 +13
==========================================
+ Hits 17388 17396 +8
- Misses 2456 2461 +5 ☔ View full report in Codecov by Sentry. |
subset_id=aperture.selected, cls=StdDevUncertainty | ||
) | ||
except ValueError: | ||
raise |
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.
raise | |
msg = SnackbarMessage( | |
f"Extracting uncertainties on subset for {aperture.selected} failed", # noqa | |
color='error', sender=self, timeout=10000) | |
self.app.hub.broadcast(msg) |
This would at least let the subset propagate to the spectral extraction tool, even if uncertainties are (initially) missing. I'd expect them to be available after the first modification (move/resize) to the subset, since I don't see the snackbar message repeated then, but could not find anything where nddate.uncertainty
is used.
2a41bed
to
c48aa92
Compare
c48aa92
to
6b0fa22
Compare
bb41b05
to
c3ef0b1
Compare
This is meant to be a compatibility patch for glue-viz/glue#2515, but currently isn't working. Opening this PR to share with SMEs @astrofrog @dhomeier. To reproduce the error I mentioned in here, open Cubeviz and try to use the Spectral Extraction plugin.