-
Notifications
You must be signed in to change notification settings - Fork 76
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
Rename monochromatic color mode and add descriptions #2894
Conversation
983fb37
to
a2f901d
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.
There are several notebooks (imviz_colorbar_mpl.ipynb, cubeviz_ndarray_gif.ipynb, imviz_dithered_gwcs.ipynb) that have references to 'monochromatic' that need to be updated.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2894 +/- ##
==========================================
- Coverage 88.71% 88.70% -0.02%
==========================================
Files 111 111
Lines 17081 17086 +5
==========================================
+ Hits 15154 15156 +2
- Misses 1927 1930 +3 ☔ View full report in Codecov by Sentry. |
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.
There are also a few references to "monochromatic" in docs/imviz/displayimages.rst that need to be updated.
Otherwise looks good to me and ill approve once those are fixed! |
@cshanahan1 - good call! Do you see anywhere else that I missed in the docs? |
@kecnry i dont think so, i dont see any more |
Changing the color mode from Colormap to Color changes the background of the image viewer from white to black. Is this expected behavior? |
If it isn't, I like the new behavior. Looks nicer in dark mode. 😆 |
Do you only see that in this PR? I believe that was from upstream changes in glue by @bmorris3 |
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.
@kecnry You're correct, it is also on main. In that case this looks good!
6ff6055
to
5a44dcc
Compare
Description
This pull request renames the "Monochromatic" color mode option to "Color" and adds descriptions within the dropdown. Setting
color_mode = 'monochromatic'
from the plot options plugin API is still supported with a deprecation message.Screen.Recording.2024-05-24.at.11.14.37.AM.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.rst
before merge. If no, maintainershould add a
no-changelog-entry-needed
label.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.