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

Sign up for GitHub

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

Jump to bottom

Exclude bad ipykernel version #1937

Merged
merged 2 commits into from
Dec 20, 2022
Merged

Exclude bad ipykernel version #1937

merged 2 commits into from
Dec 20, 2022

Conversation

rosteen
Copy link
Collaborator

@rosteen rosteen commented Dec 20, 2022

Jdaviz won't show in the notebook with ipykernel 6.19.3, pinning to avoid that version.

@rosteen rosteen added bug Something isn't working no-changelog-entry-needed changelog bot directive labels Dec 20, 2022
@rosteen rosteen added this to the 3.1.2 milestone Dec 20, 2022
pllim
pllim previously approved these changes Dec 20, 2022
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.

Can we be sure it is just that one bugfix version? If so, then LGTM. Thanks!

@codecov
Copy link

codecov bot commented Dec 20, 2022 •
edited
Loading

Codecov Report

Base: 91.72% // Head: 91.72% // No change to project coverage 👍

Coverage data is based on head (4ad430a) compared to base (d2282d2).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1937   +/-   ##
=======================================
  Coverage   91.72%   91.72%           
=======================================
  Files         140      140           
  Lines       14824    14824           
=======================================
  Hits        13597    13597           
  Misses       1227     1227

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@maartenbreddels
Copy link
Collaborator

< 6.18 should be safe for now

@pllim pllim added the 💤backport-v3.1.x on-merge: backport to v3.1.x label Dec 20, 2022
@pllim
Copy link
Contributor

pllim commented Dec 20, 2022

FWIW I didn't see problem with ipykernel 6.19.1 that was installed from conda-forge. 🤷

@maartenbreddels
Copy link
Collaborator

.1 and .2 might work, but they will give you a lot of errors in the js console (at least .2), and may not always be stable (we have reports from people on .2 where not all widgets show).

pllim
pllim reviewed Dec 20, 2022
View reviewed changes
setup.cfg Outdated Show resolved Hide resolved
@pllim pllim mentioned this pull request Dec 20, 2022
Merged
14 tasks
@rosteen @pllim
Update setup.cfg
4ad430a 
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
@rosteen
Copy link
Collaborator Author

rosteen commented Dec 20, 2022

Updated to <6.18

@pllim
Copy link
Contributor

pllim commented Dec 20, 2022

Approved. Though just to make sure, @maartenbreddels , you didn't mean <=6.18, right?

Feel free to merge when CI passes. Thanks!

@mariobuikhuizen
Copy link
Collaborator

Yes, it should be <6.18.

kecnry
kecnry approved these changes Dec 20, 2022
View reviewed changes
Copy link
Member

@kecnry kecnry left a comment

Choose a reason for hiding this comment

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

Will removing this pin be a part of #1788 or should we create a follow-up issue separately?

@rosteen
Copy link
Collaborator Author

rosteen commented Dec 20, 2022

Will removing this pin be a part of #1788 or should we create a follow-up issue separately?

Great question. I guess it depends on the timeline of the ipykernel fixes...I'll open a separate issue.

@rosteen rosteen mentioned this pull request Dec 20, 2022
Closed
@rosteen rosteen merged commit baffff9 into spacetelescope:main Dec 20, 2022
@meeseeksmachine meeseeksmachine mentioned this pull request Dec 20, 2022
Merged
meeseeksmachine pushed a commit to meeseeksmachine/jdaviz that referenced this pull request Dec 20, 2022
@rosteen @meeseeksmachine
Backport PR spacetelescope#1937: Exclude bad ipykernel version
73dbc52
@maartenbreddels
Copy link
Collaborator

maartenbreddels commented Dec 20, 2022 via email

@mariobuikhuizen
Copy link
Collaborator

Checked jdaviz with 6.19.4, and it works.

rosteen added a commit that referenced this pull request Dec 20, 2022
@rosteen
Merge pull request #1939 from meeseeksmachine/auto-backport-of-pr-193…
20e330b 
…7-on-v3.1.x

Backport PR #1937 on branch v3.1.x (Exclude bad ipykernel version)
pllim added a commit to pllim/jdaviz that referenced this pull request Dec 20, 2022
@pllim
Unpin ipykernel maxversion
75f6702 
that was pinned in spacetelescope#1937
@pllim
Copy link
Contributor

pllim commented Dec 20, 2022

Thanks! I included the pin upgrade in #1788 . FYI.

pllim added a commit to pllim/jdaviz that referenced this pull request Dec 28, 2022
@pllim
Unpin ipykernel maxversion
d861561 
that was pinned in spacetelescope#1937
pllim added a commit that referenced this pull request Jan 18, 2023
@pllim
MNT: Unpin maxversion of ipywidgets now that voila 0.4 is out, minver…
8faf482 
…sion ipywidgets now 8 (#1788)

* MNT: Unpin maxversion of ipywidgets
now that voila 0.4 is out. In fact, we pin
minversion ipywidgets to its new release.

Also bump pin for sidecar to they are all compatible.

* Ignore Widget deprecation warning

* Unpin ipykernel maxversion
that was pinned in #1937
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kecnry kecnry kecnry approved these changes

@pllim pllim pllim approved these changes

@mariobuikhuizen mariobuikhuizen mariobuikhuizen approved these changes

@duytnguyendtn duytnguyendtn Awaiting requested review from duytnguyendtn

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

@ojustino ojustino Awaiting requested review from ojustino

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

Assignees
No one assigned
Labels
bug Something isn't working no-changelog-entry-needed changelog bot directive 💤backport-v3.1.x on-merge: backport to v3.1.x
Projects
None yet
Milestone
3.1.2
Development

Successfully merging this pull request may close these issues.
5 participants
@rosteen @maartenbreddels @pllim @mariobuikhuizen @kecnry