-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Use upstream feature-flag proposal for aperture marks #7
Conversation
# To make a feature public: | ||
# * search for all instances of the feature label and remove any if-blocks, boolean traitlets | ||
# in plugins, and ultimately remove from this dictionary. | ||
feature_flags = Dict({'cone_apertures': False}).tag(sync=True) |
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.
the feature-label itself is defined here
dev_cone_support = Bool(False).tag(sync=True) | ||
ff_cone_apertures = Bool(False).tag(sync=True) |
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 creates the traitlet in the plugin so that we can use v-if
within vu
@@ -109,6 +109,8 @@ def __init__(self, *args, **kwargs): | |||
"please load data to enable this plugin." | |||
) | |||
|
|||
self._sync_feature_flag('ff_cone_apertures', 'cone_apertures') |
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 synchronizes the boolean traitlet in the plugin to get updates when the feature flag is toggled
<div v-if="aperture_selected !== 'Entire Cube' && dev_cone_support"> | ||
<div v-if="aperture_selected !== 'Entire Cube' && ff_cone_apertures"> |
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 allows hiding elements in vue when the feature is disabled
extract_plg._obj.dev_cone_support = True | ||
extract_plg._obj.wavelength_dependent = True | ||
assert mark.x[1] == before_x[1] | ||
with cubeviz_helper.app._ff_temporarily_enabled('cone_apertures'): |
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.
and lastly, tests that use the feature can use this context manager to temporarily enable them for testing. Once the feature is ready to be made public, we can either set the default to True, or preferably just search for all instances of the label, remove those lines, and remove some indentation.
553af3c
to
dea26c2
Compare
a29885b
to
6c386f1
Compare
dea26c2
to
9682af1
Compare
67e68e0
to
0dcfd1d
Compare
9682af1
to
c505591
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## aperture-marks #7 +/- ##
=================================================
Coverage ? 90.86%
=================================================
Files ? 162
Lines ? 21132
Branches ? 0
=================================================
Hits ? 19201
Misses ? 1931
Partials ? 0 ☔ View full report in Codecov by Sentry. |
3d65c12
to
d3edf57
Compare
This PR re-implements the feature-flag in spacetelescope#2664 using the proposed infrastructure up-for-debate in XXX (and therefore also includes that diff here).