-
Notifications
You must be signed in to change notification settings - Fork 79
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
MNT: Unpin maxversion of ipywidgets now that voila 0.4 is out, minversion ipywidgets now 8 #1788
Conversation
Codecov ReportBase: 91.79% // Head: 91.79% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #1788 +/- ##
=======================================
Coverage 91.79% 91.79%
=======================================
Files 140 140
Lines 14951 14951
=======================================
Hits 13724 13724
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. |
This now conflicts with the proposed change for another bug in #1791 |
7723341
to
86f59a9
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.
I confirmed that the stand-alone version of the app works with this, and it also fixes the recent problem I was seeing with the popped-out application viewer heights. The latter is a bug in 3.1, so I would advocated for changing the milestone of this to 3.1.1.
I can't quite tell if the test failure is a transient connection error downloading a file or an actual issue with dev Viola install. Maybe both? 😅 |
|
CI is all green now. |
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.
Standalone and popout windows seem to behave well for me. Not sure why one test isn't reporting its status though... but otherwise I say we get this in!
Ah, I updated the branch protection. Lemme rebase when I get a chance. Good catch! |
Did @duytnguyendtn ever have a chance to test this on Windows? Or did you already try that? If not, I'll wait till I have time to do that before merge. |
86f59a9
to
92b87a4
Compare
Timeout error is unrelated. |
92b87a4
to
3d8792c
Compare
Unfortunately, I found #1598 (comment) to still be a problem, so I am turning this back into draft. |
Alright, I started from a fresh environment and installed from this PR. I confirmed that the sidecar height bug is fixed, but popout seems completely broken for me - I get a new window, but it's blank and never populates. Could someone else see if they see the same thing? |
Something @mariobuikhuizen could look into?
On Fri, 16 Dec 2022 at 20:50, rosteen ***@***.***> wrote:
Alright, I started from a fresh environment and installed from this PR. I
confirmed that the sidecar height bug is fixed, but popout seems completely
broken for me - I get a new window, but it's blank and never populates.
Could someone else see if they see the same thing?
—
Reply to this email directly, view it on GitHub
<#1788 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANPEPNYD4AMDO6QWSFKN3TWNTBXHANCNFSM6AAAAAARQI6ETM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Maarten Breddels
Software engineer / consultant / data scientist
Python / C++ / Javascript / Jupyter
www.maartenbreddels.com / vaex.io
***@***.*** +31 6 2464 0838 <+31+6+24640838>
[image: Twitter] <https://twitter.com/maartenbreddels>[image: Github]
<https://github.com/maartenbreddels>[image: LinkedIn]
<https://linkedin.com/in/maartenbreddels>[image: Skype]
|
I'm seeing the opposite @rosteen , sidecar height is not updating accurately but popout works just fine. Clean conda environment and ipywidgets version 8.0.3. |
Interesting, I get a scrollbar on the sidecar tab so that I can see the rest of the GUI. |
@rosteen Same, is that the expected behavior? |
I thought so, but maybe we need @eteq to confirm what the expected behavior is. |
On my end (Imviz on Windows 10): ✔️Spinner shows in Link Control when click on WCS. |
e3f1b8f
to
e3e5c84
Compare
I rebased this PR, just in case. 🤞 |
e3e5c84
to
75f6702
Compare
FYI. I added the bump of ipykernel here too. Please see if your problems are gone now. Thanks! |
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.
that was pinned in spacetelescope#1937
75f6702
to
d861561
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.
I just re-tested with sidecar
and I don't see any difference between the behavior here and on main, so I think any changes to that behavior can be separate work if needed. I think we can merge this.
Okay, I'll squash-merge. We can always revert if someone finds some new problem after-the-fact. Let's not backport. Thanks! |
Description
This pull request is to fix #1613 and fix #1787 and fix #1926 and fix #1938
xref #1791 .
Multiple devs should check that things are truly fixed by this new voila release since several things were broken before. Make sure your ipywidgets is at least v8 and voila is v0.4 when you test.
Blocked by
Bugs check list
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.