Skip to content
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

Jump to bottom

fix plot options multi to single select #2505

Merged
merged 1 commit into from
Oct 11, 2023
Merged

fix plot options multi to single select #2505

merged 1 commit into from
Oct 11, 2023

Conversation

kecnry
Copy link
Member

@kecnry kecnry commented Oct 11, 2023

Description

This pull request fixes the case where the callback on the change to the multiselect traitlet was being processed before the default value was changed from a list to a string resulting in the following behavior:

Screen.Recording.2023-10-11.at.4.44.05.PM.mov

Change log entry

  • Is a change log needed? If yes, is it added to 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, maintainer
    should 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.

  • Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
  • Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
  • Do the proposed changes follow the STScI Style Guides?
  • Are tests added/updated as required? If so, do they follow the STScI Style Guides?
  • Are docs added/updated as required? If so, do they follow the STScI Style Guides?
  • Did the CI pass? If not, are the failures related?
  • Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
  • After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

@kecnry kecnry added bug Something isn't working trivial Only needs one approval instead of two labels Oct 11, 2023
@kecnry kecnry added this to the 3.7.1 milestone Oct 11, 2023
@kecnry
fix plot options multi->single select
2c1f528 
* previously the traitlets could get updated out of order - its more reliable here to just check the type rather than the switch itself
@kecnry kecnry force-pushed the fix-plot-options-multi-to-single branch from 8dd32c8 to 2c1f528 Compare October 11, 2023 20:47
@kecnry kecnry marked this pull request as ready for review October 11, 2023 20:47
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...nfigs/default/plugins/plot_options/plot_options.py 82.93% <66.66%> (-0.18%) ⬇️

📢 Thoughts on this report? Let us know!.

@pllim pllim added the 💤 backport-v3.7.x on-merge: backport to v3.7.x label Oct 11, 2023
pllim
pllim approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might explain the cryptic stuff that popped up in the snackbar when I was reviewing the other PR but didn't have time to investigate. Thanks!

@kecnry kecnry merged commit 14b61e5 into spacetelescope:main Oct 11, 2023
@kecnry kecnry deleted the fix-plot-options-multi-to-single branch October 11, 2023 21:06
meeseeksmachine pushed a commit to meeseeksmachine/jdaviz that referenced this pull request Oct 11, 2023
@kecnry @meeseeksmachine
Backport PR spacetelescope#2505: fix plot options multi to single select
bd9814d
@meeseeksmachine meeseeksmachine mentioned this pull request Oct 11, 2023
Merged
pllim added a commit that referenced this pull request Oct 11, 2023
@pllim
Merge pull request #2507 from meeseeksmachine/auto-backport-of-pr-250…
34ce485 
…5-on-v3.7.x

Backport PR #2505 on branch v3.7.x (fix plot options multi to single select)
rosteen pushed a commit to rosteen/jdaviz that referenced this pull request Oct 19, 2023
@kecnry @rosteen
fix plot options multi->single select (spacetelescope#2505)
89f2d2b 
* previously the traitlets could get updated out of order - its more reliable here to just check the type rather than the switch itself
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@pllim pllim pllim approved these changes

@rosteen rosteen Awaiting requested review from rosteen rosteen is a code owner

@javerbukh javerbukh Awaiting requested review from javerbukh javerbukh is a code owner

@bmorris3 bmorris3 Awaiting requested review from bmorris3 bmorris3 is a code owner

@haticekaratay haticekaratay Awaiting requested review from haticekaratay haticekaratay is a code owner

@cshanahan1 cshanahan1 Awaiting requested review from cshanahan1 cshanahan1 is a code owner

Assignees
No one assigned
Labels
bug Something isn't working trivial Only needs one approval instead of two 💤 backport-v3.7.x on-merge: backport to v3.7.x
Projects
None yet
Milestone
3.7.1
Development

Successfully merging this pull request may close these issues.
2 participants
@kecnry @pllim