replace voila with solara for CLI/standalone app #2909

kecnry · 2024-06-05T10:58:03Z

Description

This pull request replaces the role of voila with solara for launching the standalone/CLI version of jdaviz. Note that we don't currently have a generic (config-agnostic) icon, so this currently uses the cubeviz icon for the page favicon.

Todo before merge:

fix matplotlib incompatibility issue
- update pin of solara on next release
fix snackbar logger overlay
pin release of ipypopout that does not pull in voila
stretch or follow-up: window titles for popouts

(likely) closes #2268

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. Bugfix milestone also needs an accompanying backport label.
After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

codecov · 2024-06-07T14:28:27Z

Codecov Report

Attention: Patch coverage is 33.33333% with 50 lines in your changes missing coverage. Please review.

Project coverage is 88.84%. Comparing base (d71dc91) to head (a8443cc).
Report is 2 commits behind head on main.

Files	Patch %	Lines
jdaviz/solara.py	41.66%	28 Missing ⚠️
jdaviz/cli.py	0.00%	17 Missing ⚠️
jdaviz/core/launcher.py	50.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2909      +/-   ##
==========================================
+ Coverage   88.82%   88.84%   +0.02%     
==========================================
  Files         112      113       +1     
  Lines       17569    17612      +43     
==========================================
+ Hits        15605    15647      +42     
- Misses       1964     1965       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* currently has some styling issues and does not support hotreloading properly * still needs some updates to testing, docs, etc

* fix: height and spacing * fix: height of launcher

Use the new class that is also available in a Solara popout window. Keep the styling of #popout-widget-container for backward compatibility with ipypopout<1.3.0.

* feat: use solara for pyinstaller * rich.logging hidden import * make test run * close when tab is closed

could eventually dynamically change this as configs are selected, data loaded, etc.

pllim · 2024-08-15T18:45:19Z

Re: Compass crash -- It is not a problem on main using voila 0.4.5.

Also, if threading is a problem, Cubeviz movie maker might also be in trouble.

kecnry · 2024-08-15T18:46:13Z

moving back to draft since there are now likely two merge-blockers (matplotlib issue and pinning ipypopout to a newer version without voila as a dep). Reviews should be able to continue on everything else though!

bmorris3

Launching standalone, loading a spectral cube, and clicking the snackbar logger grays out the interface, before crashing in the terminal with Aborted.:

Any ideas what's up there?

kecnry · 2024-08-16T17:32:03Z

nope, but I can reproduce. I'll add to the list of blockers - no obvious traceback or warnings in the console, so this one might be a bit tricky to track down.

no longer lists voila as a dep

maartenbreddels · 2024-08-20T13:00:49Z

from https://docs.python.org/3/library/faulthandler.html - you can set PYTHONFAULTHANDLER=1 to get a stacktrace of when the crash happened, that might help pinpoint the issue.

Normally, setting MPLBACKEND will set the backend because importing matplotlib will set the backend based on this environment variable. However, in the case where matplotlib is imported before solara, such as jdaviz is doing, we need to set the backend manually. Related: spacetelescope/jdaviz#2909

maartenbreddels · 2024-08-20T17:00:49Z

widgetti/solara#741 should fix the matplotlib issue, this was an issue on solara's side (together how the order of importing is happening).

kecnry · 2024-08-20T17:37:24Z

widgetti/solara#741 should fix the matplotlib issue

Can confirm that it fixes it on my end. I'll bump the requirement of solara here once I see that is merged & released.

Normally, setting MPLBACKEND will set the backend because importing matplotlib will set the backend based on this environment variable. However, in the case where matplotlib is imported before solara, such as jdaviz is doing, we need to set the backend manually. Related: spacetelescope/jdaviz#2909

kecnry · 2024-08-21T17:33:19Z

all known issues have now been resolved so I'm marking this as ready for review again. Make sure to update solara to the updated pin of 1.37.2 or later before testing as that fixes the matplotlib/compass issues.

pllim · 2024-08-21T18:48:38Z

What about the standalone builds?

kecnry · 2024-08-21T18:50:42Z

What about the standalone builds?

These are not currently building on main either. @rosteen - do you think anything here needs to be changed to support that? Does this PR make the situation worse?

pllim · 2024-08-21T19:04:28Z

I thought Solara would be magical enough to also fix the standalone problems that were choking up voila. Perhaps I misunderstood?

kecnry · 2024-08-21T19:38:03Z

I cannot seem to start this from Windows (not WSL2, but natively on Windows).

Can you please help debug as I have not seen that? os.environ['JDAVIZ_START_DIR'] is still being populated here, but it seems either that or local_path is still None.

pllim

Standalone builds still not working aside, I like the code simplification. Thanks!

pllim · 2024-08-21T19:52:15Z

The path stuff... Not sure what happened, I cannot reproduce it now. 🤷‍♀️

bmorris3

Thanks for the fixes, this is great!

* replace voila with solara in cli * feat: use solara for pyinstaller * remove no longer needed share/jupyter/nbconvert directories * set page title to "Jdaviz" * override favicon (currently with cubeviz icon) * cli: remove unneeded logic for python<3.10 * update/remove other mentions of voila across codebase * add solara to devdeps * bump ipypopout --------- Co-authored-by: Mario Buikhuizen <[email protected]> Co-authored-by: Maarten Breddels <[email protected]>

kecnry force-pushed the standalone-solara branch 2 times, most recently from 5686221 to 2bdd788 Compare June 5, 2024 12:12

kecnry added this to the 4.0 milestone Jun 5, 2024

kecnry force-pushed the standalone-solara branch 2 times, most recently from f3d6389 to a829df0 Compare June 7, 2024 14:04

kecnry force-pushed the standalone-solara branch from cbeede1 to fda0b98 Compare July 9, 2024 14:46

kecnry force-pushed the standalone-solara branch from af83044 to adb00cc Compare July 30, 2024 14:35

kecnry added UI/UX😍 Build standalone Additional pylons labels Aug 15, 2024

kecnry changed the title ~~WIP: replace voila with solara for CLI/standalone app~~ replace voila with solara for CLI/standalone app Aug 15, 2024

kecnry and others added 12 commits August 15, 2024 12:21

replace voila with solara in cli

c4b2beb

* currently has some styling issues and does not support hotreloading properly * still needs some updates to testing, docs, etc

remove unneeded voila stuff

5930cf6

fix: height and spacing (#9)

d1b2de0

* fix: height and spacing * fix: height of launcher

fix: make the app full height in popout window (#12)

1b0ce77

Use the new class that is also available in a Solara popout window. Keep the styling of #popout-widget-container for backward compatibility with ipypopout<1.3.0.

feat: use solara for pyinstaller (#11)

73a7b9d

* feat: use solara for pyinstaller * rich.logging hidden import * make test run * close when tab is closed

re-enable standalone build check

b29b872

remove no longer needed share/jupyter/nbconvert directories

5b160f6

hide solara tagline

784f81f

set page title to "Jdaviz"

fa13344

could eventually dynamically change this as configs are selected, data loaded, etc.

override favicon (currently with cubeviz icon)

fc3c051

cli: remove unneeded logic for python<3.10

82ddb49

changelog entry

9042937

kecnry force-pushed the standalone-solara branch from a1d3df7 to 9042937 Compare August 15, 2024 16:21

kecnry marked this pull request as ready for review August 15, 2024 16:24

kecnry requested review from rosteen, javerbukh, pllim, bmorris3 and haticekaratay as code owners August 15, 2024 16:24

kecnry marked this pull request as draft August 15, 2024 18:46

bmorris3 reviewed Aug 16, 2024

View reviewed changes

bump ipypopout

a8443cc

no longer lists voila as a dep

maartenbreddels mentioned this pull request Aug 20, 2024

fix: if matplotlib was imported before solara, we set the backend widgetti/solara#741

Merged

kecnry added 3 commits August 21, 2024 12:47

fix logger overlay

556d75f

improved theme handling

472719a

update solara pin for mpl fix

55efd71

kecnry marked this pull request as ready for review August 21, 2024 17:33

This comment was marked as resolved.

Sign in to view

pllim approved these changes Aug 21, 2024

View reviewed changes

kecnry added the Ready for final review label Aug 21, 2024

bmorris3 approved these changes Aug 22, 2024

View reviewed changes

kecnry merged commit 3fc9f8d into spacetelescope:main Aug 22, 2024
19 of 25 checks passed

kecnry deleted the standalone-solara branch August 22, 2024 19:33

kecnry mentioned this pull request Oct 15, 2024

Fix popouts without voila #3218

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

replace voila with solara for CLI/standalone app #2909

replace voila with solara for CLI/standalone app #2909

kecnry commented Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 7, 2024 •

edited

Loading

pllim commented Aug 15, 2024

kecnry commented Aug 15, 2024

bmorris3 left a comment

kecnry commented Aug 16, 2024

maartenbreddels commented Aug 20, 2024

maartenbreddels commented Aug 20, 2024

kecnry commented Aug 20, 2024

kecnry commented Aug 21, 2024

pllim commented Aug 21, 2024

kecnry commented Aug 21, 2024

pllim commented Aug 21, 2024

This comment was marked as resolved.

kecnry commented Aug 21, 2024

pllim left a comment

pllim commented Aug 21, 2024

bmorris3 left a comment

replace voila with solara for CLI/standalone app #2909

replace voila with solara for CLI/standalone app #2909

Conversation

kecnry commented Jun 5, 2024 • edited Loading

Description

Change log entry

Checklist for package maintainer(s)

codecov bot commented Jun 7, 2024 • edited Loading

Codecov Report

pllim commented Aug 15, 2024

kecnry commented Aug 15, 2024

bmorris3 left a comment

Choose a reason for hiding this comment

kecnry commented Aug 16, 2024

maartenbreddels commented Aug 20, 2024

maartenbreddels commented Aug 20, 2024

kecnry commented Aug 20, 2024

kecnry commented Aug 21, 2024

pllim commented Aug 21, 2024

kecnry commented Aug 21, 2024

pllim commented Aug 21, 2024

This comment was marked as resolved.

kecnry commented Aug 21, 2024

pllim left a comment

Choose a reason for hiding this comment

pllim commented Aug 21, 2024

bmorris3 left a comment

Choose a reason for hiding this comment

kecnry commented Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 7, 2024 •

edited

Loading