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.

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

Fix Trame Viewers in Jupyter #4844

Merged
merged 4 commits into from
Sep 5, 2023
Merged

Fix Trame Viewers in Jupyter #4844

merged 4 commits into from
Sep 5, 2023

Conversation

annehaley
Copy link
Contributor

@annehaley annehaley commented Sep 4, 2023

Overview

This PR is intended to address any oversights from #4811.
Related to #4835.

Changes

  • Initialize UI-related state variables in the constructor of the BaseViewer class. This makes calling the ui function not mandatory for using the viewer.

@github-actions github-actions bot added the bug Uh-oh! Something isn't working as expected. label Sep 4, 2023
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #4844 (123a8d6) into main (66fc5f8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4844   +/-   ##
=======================================
  Coverage   95.78%   95.78%           
=======================================
  Files         130      130           
  Lines       21300    21305    +5     
=======================================
+ Hits        20402    20408    +6     
+ Misses        898      897    -1     

@tkoyama010
Copy link
Member

Thanks @annehaley for the quick PR. And sorry that we didn't test this in our Jupyter environment. I will release 0.42.1 when this PR is merged.

@johannes-mueller
Copy link

LGTM. Thanks for the quick fix @annehaley

@larsoner
Copy link
Contributor

larsoner commented Sep 5, 2023

I was about to open an issue about #4811 breaking things based on a git bisect but I can confirm that this PR seems to fix them!

@tkoyama010 tkoyama010 linked an issue Sep 5, 2023 that may be closed by this pull request
@tkoyama010
Copy link
Member

@annehaley When you are ready to merge, could you please push the ready for review button?

@annehaley annehaley marked this pull request as ready for review September 5, 2023 23:55
@tkoyama010 tkoyama010 merged commit f0db627 into main Sep 5, 2023
25 checks passed
@tkoyama010 tkoyama010 deleted the fix/trame-viewer-defaults branch September 5, 2023 23:59
tkoyama010 pushed a commit that referenced this pull request Sep 6, 2023
* Add default values for state variables before ui execution

* Pass `server` as arg to `get_viewer` in jupyter `initialize`

* Fix tests: dont expect a "no data" error from `viewer.export()`
@tkoyama010 tkoyama010 mentioned this pull request Sep 6, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Uh-oh! Something isn't working as expected.
Projects
None yet
4 participants