Visitar URL original
Enable Binder by MarcSkovMadsen · Pull Request #2198 · holoviz/panel · GitHub
Skip to content

Enable Binder - #2198

Merged
philippjfr merged 71 commits into
masterfrom
binder
May 12, 2021
Merged

philippjfr merged 71 commits into
masterfrom
binder

Conversation

@MarcSkovMadsen

@MarcSkovMadsen MarcSkovMadsen commented Apr 17, 2021 •

Copy link
Copy Markdown
Collaborator

Address #1500, #1369 (comment) and #2218.

This will enable opening the Panel examples on Binder. Whether it will also be possible to add a binder link to all examples is unknown and "nice to have" for this PR.

image

For now you can access binder via https://mybinder.org/v2/gh/holoviz/panel/binder?urlpath=lab/tree/examples

I would like a suggestion from @philippjfr or @jbednar which link we should provide when released. I see the following options

  • Link to binder branch.
    • Pros: We can control what is available on Binder
    • Cons: We will have to remember and spend time on updating the binder branch
  • Link to master branch.
    • Pros: Everything is always the latest
    • Cons: There might be things is master that changes the behaviour of the current version of Panel released.
  • Link to release branch of Panel. I.e. for example the version for the Panel 0.11.3 release
    • Pros: The current version of Panel is available on Binder
    • Cons:
      • Will have to remember to update links across the repository.
      • Users having bookmarked the link would stay on that version of Panel on Binder

@MarcSkovMadsen

Copy link
Copy Markdown
Collaborator Author

Status: Things are starting to work. I'm finding bugs in the Reference Examples. I have to update the gallery notebooks in order to be able to serve them as apps. I would like to configure VS Code to enable showcasing how powerful the combination of Panel and VS Code is.... Also on Binder and in a Jupyter Hub.

panel-binder.mp4

So I would really, really like Panel to work in VS Code. But it requires fixing bokeh/jupyter_bokeh#131

@MarcSkovMadsen

MarcSkovMadsen commented Apr 20, 2021 •

Copy link
Copy Markdown
Collaborator Author

Hi @philippjfr

As I see it 1) Binder is working 2) It now comes with a nice setup like panel-jupyter-proxy, panel serve of all gallery examples, vs code integrated and a new, improved index page.

In principle this could be all for this PR except adding a link to binder in different places. For example the README.

BUT all the notebooks need to be walked through and prettified in order to be useable as apps. This could be a lot of work and a lot of minor changes. And to be honest. Even without binder a test and update of the notebooks could be in place (c.f. all the bugs posted, I have just scratched the surface I believe).

Here are my questions.

  • Do you want the gallery notebooks updated in general to be responsive and end up in a nice app template?
  • Do you want the reference examples updated in general to be responsive and end up in a nice app template?`
  • If yes to some of these questions. Do you want separate PRs for each notebook, one large Notebook PR or just this PR with everything?
  • Could you answer the question about which link to provide to users posted in the first post? Thanks.

@philippjfr

Copy link
Copy Markdown
Member

Do you want the gallery notebooks updated in general to be responsive and end up in a nice app template?

No, some of them sure, but definitely not all. The focus should be the feature being demonstrated not making all the examples look like.

Do you want the reference examples updated in general to be responsive and end up in a nice app template?`

Definitely not, in fact I'd argue there's no reason to make these available as apps at all.

Could you answer the question about which link to provide to users posted in the first post? Thanks.

I'll make sure to configure this once you're done here. The link will go in the interactivity warning that floats on the right side.

@philippjfr

Copy link
Copy Markdown
Member

If yes to some of these questions. Do you want separate PRs for each notebook, one large Notebook PR or just this PR with everything?

Just make a single PR for any coordinated changes like this.

@MarcSkovMadsen

MarcSkovMadsen commented May 2, 2021 •

Copy link
Copy Markdown
Collaborator Author

Binder is now working as I originally intended it to and more :-).

Some things are not in scope for me

  • Adding links from every example at panel.holoviz.org to the notebook and app on Binder. I hope @philippjfr can/ will do this.

Some things are not working and I am not capable of getting it working

Working Video

binder-gallery-works.mp4

Not Working

Watch this video on Youtube https://www.youtube.com/watch?v=gI3qQRVdIIE

@philippjfr Feel free to review and start creating value from this :-) I don't plan to do more on this before getting a review and feedback.

Thanks.

@MarcSkovMadsen MarcSkovMadsen changed the title Enable Binder - WORK IN PROGRESS Enable Binder May 2, 2021
@MarcSkovMadsen
MarcSkovMadsen requested a review from philippjfr May 2, 2021 09:31

@jbednar jbednar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! This will make things look and feel much better. Should we promote the approach from #2271 or is it better to put the template separately like it is here?

Comment thread binder/start
Xvfb :99 -screen 0 1280x1024x24 > /dev/null 2>&1 &
sleep 3
jupyter trust examples/**/*.ipynb
jupyter trust examples/**/**/*.ipynb

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not a bash expert, but doesn't ** indicate recursive directory searching nowadays? If so, isn't the second trust call redundant?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I thought so. But it did not work like that for me. So I added it twice. That works.

Comment thread examples/gallery/apis/stocks_altair.ipynb Outdated
@MarcSkovMadsen

Copy link
Copy Markdown
Collaborator Author

Nice! This will make things look and feel much better. Should we promote the approach from #2271 or is it better to put the template separately like it is here?

When the new approach #2271 is ready and tested I think it would make a lot of sense to switch. That syntax is nice and shorter.

The only downside is that then not a lot of examples will show how to build apps with templates for non-notebook users. There the natural workflow might still be to explicitly declare a template ??

@jbednar

jbednar commented May 3, 2021 •

Copy link
Copy Markdown
Member

Good point. We should still have a separate example of building such a template explicitly somewhere, and while it's true people may not see that example, having people use servable unnecessarily in a standalone file is not that terrible. I.e., I agree it doesn't make as much sense outside a notebook, but it's still going to be fine for many people, and so I'm ok with more advanced non-notebook users having to do a bit more work to find that there is this other way.

Comment thread setup.py Outdated
Comment on lines +153 to +157
'pydeck',
'pyecharts',
'idom',
'xlsxwriter',
'pyarrow',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Most of these cannot be added to setup.py since that is also the ground truth for conda installs. If I remove these here will this break the binder setup?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes. But we can add them to one of the files in the binder subfolder. Can you point out which should not be included?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Already removed all the ones you added. Anything that is not conda installable cannot appear here, so for now I've removed pydeck, pyecharts, idom, xlsxwriter and pyarrow. You could readd pyarrow since that is available.

Comment thread setup.py Outdated
philippjfr and others added 3 commits May 3, 2021 12:05
Co-authored-by: James A. Bednar <jbednar@users.noreply.github.com>
@codecov

codecov Bot commented May 3, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #2198 (bed6e07) into master (9984f55) will increase coverage by 0.38%.
The diff coverage is 41.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2198      +/-   ##
==========================================
+ Coverage   83.64%   84.02%   +0.38%     
==========================================
  Files         183      181       -2     
  Lines       21947    21940       -7     
==========================================
+ Hits        18357    18436      +79     
+ Misses       3590     3504      -86     
Impacted Files Coverage Δ
panel/tests/pane/test_alert.py 90.90% <0.00%> (ø)
...nel/tests/template/fast/test_fast_grid_template.py 95.83% <0.00%> (-2.09%) ⬇️
...nel/tests/template/fast/test_fast_list_template.py 87.50% <0.00%> (-6.25%) ⬇️
panel/tests/template/test_manual.py 61.61% <0.00%> (-1.39%) ⬇️
panel/tests/template/test_vanilla_manual.py 86.95% <0.00%> (-2.41%) ⬇️
panel/tests/widgets/test_speech_to_text.py 90.72% <0.00%> (ø)
panel/tests/widgets/test_text_to_speech.py 92.50% <0.00%> (ø)
panel/tests/pane/test_markup.py 99.33% <100.00%> (ø)
panel/tests/widgets/test_tables.py 99.23% <100.00%> (ø)
panel/config.py 51.78% <0.00%> (-0.16%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9984f55...bed6e07. Read the comment docs.

@MarcSkovMadsen

MarcSkovMadsen commented May 3, 2021 •

Copy link
Copy Markdown
Collaborator Author

There is one fundamental thing to consider. Which branch should be promoted and shared with users? In principle any branch will be available on Binder. But it's important to agree on one that is promoted.

My recommendation would be to promote the binder branch. Then it can be updated and fixed whenever there is a new release. The master branch for example will not always be working is my experience. For example the .ts code cannot build right now I believe.

@philippjfr

Copy link
Copy Markdown
Member

For example the .ts code cannot build right now I believe.

There are a few errors but they are not preventing the .ts from being built and don't cause any real issues. However I should add a CI check to ensure there are zero TS errors or warnings. Not entirely excited about a binder branch since that's an additional release step but given all the alternatives that still seems like the best approach.

@MarcSkovMadsen

Copy link
Copy Markdown
Collaborator Author

The alternative is just using the master branch. But then users would see functionality that is not yet released. That might be confusing. But maybe also just fine.

@jbednar

jbednar commented May 3, 2021

Copy link
Copy Markdown
Member

Sound to me like it ought to be using a tag indicating the latest release, rather than a separate branch? Our release workflow would then need to update latest-release or released whenever we make a full (non-dev) release. Seeing changes post-release would not only be confusing, it would likely not work, if it's anything like when I try Panel outside the release process (due to the JS build/publish process involved).

Comment thread panel/tests/widgets/test_tables.py Outdated
@philippjfr
philippjfr merged commit 985a180 into master May 12, 2021
@philippjfr
philippjfr deleted the binder branch May 12, 2021 09:50
@philippjfr

Copy link
Copy Markdown
Member

Thanks again for this extraordinary effort @MarcSkovMadsen. I'll be adding binder links to all pages in a separate PR.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants