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

Add ability to specify binder environment url along with notebook #464

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jsheunis
Copy link
Member

Closes #459

  • The 'binder_env_url' field is introduced into the 'notebooks' field of the dataset schema, to allow a binder environment to be specified along with a notebook.
  • This URL will replace the base environment url that was previously hardcoded; while the base url remains as the default.
  • A fix is added to the flag that sets whether the binder button is displayed or not.
  • The palmerpenguins dataset metadata is updated to include some arbitrary binder_env_url in order to test the new functionality
  • An HTMLescape function is added to encode notebook names with e.g. spaces correctly for HTML.

- The 'binder_env_url' field is introduced into the 'notebooks' field
  of the dataset schema, to allow a binder environment to be specified
  along with a notebook.
- This URL will replace the base environment url that was previously
  hardcoded; while the base url remains as the default.
- A fix is added to the flag that sets whether the binder button
  is displayed or not.
- The palmerpenguins dataset metadata is updated to include some
  arbitrary binder_env_url in order to test the new functionality
- An HTMLescape function is added to encode notebook names with
  e.g. spaces correctly for HTML.
Copy link

netlify bot commented May 28, 2024

Deploy Preview for datalad-catalog canceled.

Name Link
🔨 Latest commit 6c8fb72
🔍 Latest deploy log https://app.netlify.com/sites/datalad-catalog/deploys/66565d0d5d585f000834943d

Comment on lines +770 to +782
if (notebook.hasOwnProperty("binder_env_url") && notebook["binder_env_url"]) {
environment_url = notebook["binder_env_url"]
}
}

binder_url =
environment_url +
"?urlpath=git-pull%3Frepo%3D" +
content_url +
"%26urlpath%3Dnotebooks%252F" +
content_repo_name +
"%252F" +
notebook_name +
"%3Frepourl%3D%22" +
dataset_url +
"%22";
notebook_name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you are trying to do too much here, and end up being too restrictive.

AFAIK, the ?urlpath=git-pull and probably the following bits come from nbgitpuller, which pulls the notebook into the defined environment. This is in general considered good practice (because changing just the notebook does not necessitate binder container rebuild), but what if I have my binder link ready and do not want to use the nbgitpuller?

I do not fully parse the other components at the moment, but it looks like there may be further assumptions.

In my current example, I have a repo with environment and notebook. I generated the Binder URL (https://mybinder.org/v2/gh/sfb1451/rostami_etal_2024-demo/HEAD?labpath=Demo.ipynb) with the help of https://mybinder.org/. If I were more familiar with nbgitpuller and wanted to use it, I would probably be able to generate a matching URL too.

So... why not just have a metadata property for Binder URL?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with your observations, and I'd add that I think both should be an option, i.e.:

  • have a metadata property on the dataset schema that allows to specify a fully self-contained binder url, which would open an environment that contains the relevant notebook(s)
  • have the property on a per-notebook level as proposed in this PR.

mslw added a commit to sfb1451/metadata-catalog that referenced this pull request May 30, 2024
This makes it possible to specify a fully custom URL for Binder,
without having to use datalad-binder as the "environment" repo.

This applies yet-unmerged changes from
datalad/datalad-catalog/pull/464

Technical note: done by downloading a PR patch from GitHub, (add
.patch to URL), replacing file paths, editing it by hand to exclude
commits and changes to irrelevant files, and applying it with
Git (patch - apply plain patch in magit).

Co-authored-by: Stephan Heunis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow binder environment to be specified as metadata
2 participants