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

Don't try to update viewer state if it was deleted #2541

Merged
merged 6 commits into from
Nov 1, 2023
Merged

Don't try to update viewer state if it was deleted #2541

merged 6 commits into from
Nov 1, 2023

Conversation

rosteen
Copy link
Collaborator

@rosteen rosteen commented Oct 30, 2023

Closes #2521 , turned out to be very simple once I tracked it down. This was previously causing an error if the viewer was deleted, since None has no state.

@rosteen rosteen added bug Something isn't working plugin Label for plugins common to multiple configurations labels Oct 30, 2023
@rosteen rosteen added this to the 3.7.2 milestone Oct 30, 2023
@pllim
Copy link
Contributor

pllim commented Oct 30, 2023

Add a test? 😉

@codecov
Copy link

codecov bot commented Oct 30, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
...nfigs/default/plugins/plot_options/plot_options.py 91.22% <100.00%> (-0.78%) ⬇️
jdaviz/configs/imviz/tests/test_viewers.py 100.00% <100.00%> (ø)

📢 Thoughts on this report? Let us know!.

@rosteen
Copy link
Collaborator Author

rosteen commented Oct 31, 2023

Add a test? 😉

No.

...ok, fiiine 😆

@rosteen
Add test
933650d 
Add test

Fix codestyle

codestyle
@rosteen rosteen force-pushed the let-it-go-plot-options branch from b09a311 to 933650d Compare November 1, 2023 13:22
@rosteen
Copy link
Collaborator Author

rosteen commented Nov 1, 2023 •
edited
Loading

I'm having trouble writing a test that fails on main. The code from the test I added here results in an error traceback on main if run in a notebook context, but passes as a test. Not sure what's missing from the test to get it to fail...

Edit: wait, I lied, in the notebook I was using real images linked by WCS rather than the numpy arrays. If I load the numpy arrays in the notebook it doesn't error 🤔.

@rosteen
Copy link
Collaborator Author

rosteen commented Nov 1, 2023

As discussed offline, going to merge this rather than spending a bunch more time figuring out a test that fails on main. Codecov also seems to be giving a wonky result.

@rosteen rosteen merged commit 309b76b into spacetelescope:main Nov 1, 2023
@meeseeksmachine meeseeksmachine mentioned this pull request Nov 1, 2023
Merged
meeseeksmachine pushed a commit to meeseeksmachine/jdaviz that referenced this pull request Nov 1, 2023
@rosteen @meeseeksmachine
Backport PR spacetelescope#2541: Don't try to update viewer state if …
f72981e 
…it was deleted
pllim added a commit that referenced this pull request Nov 1, 2023
@pllim
Merge pull request #2544 from meeseeksmachine/auto-backport-of-pr-254…
78933b8 
…1-on-v3.7.x

Backport PR #2541 on branch v3.7.x (Don't try to update viewer state if it was deleted)
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@kecnry kecnry kecnry approved these changes

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

@pllim pllim Awaiting requested review from pllim pllim 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 plugin Label for plugins common to multiple configurations
Projects
None yet
Milestone
3.7.2
Development

Successfully merging this pull request may close these issues.

Plot Option breaks when viewer deleted while selected
3 participants
@rosteen @pllim @kecnry