-
Notifications
You must be signed in to change notification settings - Fork 9
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
Flatten: separate previews for flattened and trend #41
Conversation
aa8749d
to
9fd0a51
Compare
e75a649
to
d3c51f3
Compare
ae1e5aa
to
c49f5e9
Compare
Codecov ReportAll modified lines are covered by tests ✅
📢 Thoughts on this report? Let us know!. |
c49f5e9
to
4f53454
Compare
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.
Looks good!
@@ -43,6 +46,7 @@ class Flatten(PluginTemplateMixin, DatasetSelectMixin, AddResultsMixin): | |||
uses_active_status = Bool(True).tag(sync=True) | |||
|
|||
show_live_preview = Bool(True).tag(sync=True) | |||
show_trend_preview = Bool(True).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.
show_trend_preview = Bool(True).tag(sync=True) | |
show_trend_preview = Bool(False).tag(sync=True) |
Take or leave this suggestion, but I think the default should be less cluttered and redundant. Especially since the trend color and live preview color are identical, there's a lot to see in the viewer if this is enabled by default.
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.
usually I actually prefer to see the trend over the preview 🤷♂️ Let's leave for now and revisit as we use it more.
Co-authored-by: Brett M. Morris <[email protected]>
NOTE: This PR should be rebased after #38 is merged (currently builds on top of it to avoid needing to re-implement), and therefore needs to wait for jdaviz 3.7 to be released.This PR separates the plugin-settings switch for live-previews in the flattening plugin to control the (unnormalized) flattened light curve and the trend separately. In cases where there is not much flattening to be done, it is difficult to see the trend and ensure it is correct if the flattened light curve is directly on top.
Screen.Recording.2023-10-19.at.9.47.14.AM.mov