feature-flagged wireframe for cubeviz spec extraction plan #2676
Conversation
github-actions
bot
added
cubeviz
plugin
kecnry
force-pushed
the
cube-spec-extract-wireframe
branch
from
648917f
to
3aad943
Compare
kecnry
force-pushed
the
cube-spec-extract-wireframe
branch
from
3aad943
to
711bc79
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
==========================================
- Coverage 90.85% 90.84% -0.02%
==========================================
Files 162 162
Lines 21126 21152 +26
==========================================
+ Hits 19195 19215 +20
- Misses 1931 1937 +6 ☔ View full report in Codecov by Sentry. |
kecnry
requested review from
rosteen,
javerbukh,
pllim,
bmorris3,
haticekaratay,
cshanahan1 and
gibsongreen
as code owners
There was a problem hiding this comment.
Works as expected and the hidden features give me a good idea of how to build #2679 on top of this. Lets get this merged so we can get the entire spectral extraction effort done and go through it with a fine-tooth comb afterward.
| bg_scale_factor = Float(1).tag(sync=True) | ||
| bg_wavelength_dependent = Bool(False).tag(sync=True) | ||
|
|
||
| subpixel = Bool(False).tag(sync=True) |
There was a problem hiding this comment.
Reminder: let's change references to subpixel to something which is not the name of one of the three aperture masking methods ({exact, subpixel, center}). It's clunky but accurate to call these "aperture masking methods."
There was a problem hiding this comment.
I'll leave this up to #2679 to modify (with no objections if we do in fact go with three methods instead of just two).
| if self.dev_bg_support: | ||
| expose += ['background', 'bg_wavelength_dependent'] | ||
| if self.dev_subpixel_support: | ||
| expose += ['subpixel'] |
| </j-tooltip> | ||
| <v-switch | ||
| v-model="wavelength_dependent" | ||
| label="Wavelength dependent" |
There was a problem hiding this comment.
Should we label this "Conical (wavelength dependent)"?
There was a problem hiding this comment.
Let's run this by POs and can also update in any of the follow-up works... technically it won't look like a cone in frequency-space or if there are gaps in wavelength... but I agree that the terminology may still be more descriptive at first glance to a user.
There was a problem hiding this comment.
(@javerbukh - feel free to change this as part of #2679 as well)
| <v-switch | ||
| v-model="subpixel" | ||
| label="Subpixel" | ||
| hint="Extract spectrum using a subpixel aperture in place of the subset mask" |
There was a problem hiding this comment.
(Replace this subpixel please 🙏)
…scope#2676) * feature-flagged wireframe for cubeviz spec extraction plan * changelog entry * fix original mark visibility
Description
This pull request implements the basic UI framework for planned spectral extraction work in cubeviz, including:
All of these are currently behind "feature-flags" so do not show for the user if/when merged into main. As each individual feature is completed, we can remove the feature flag.
Changes that this makes to main now:
Without feature flags (ie. how users will see this now):
Screen.Recording.2024-01-25.at.3.44.14.PM.mov
With feature flags (plan for the future of spectral extraction, but non-functional):
Screen.Recording.2024-01-25.at.3.45.00.PM.mov
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.