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 binder links #93

Merged
merged 6 commits into from
Oct 26, 2020
Merged

Add binder links #93

merged 6 commits into from
Oct 26, 2020

Conversation

sroet
Copy link
Collaborator

@sroet sroet commented Oct 23, 2020

after pr #92
This PR adds the repective binder links to the

  • README.md
  • examples page of the documentation
  • add a link to binder for every notebook
  • enable jinja2 rendering for all .rst your docs

(@dwhswenson any place missing? for example I can add a link to each ipynb that would start binder for themself)

@dwhswenson
Copy link
Owner

I think README and examples should be enough. BTW, part of my upcoming examples refresh involves adding numbers to the notebooks. That will:

  1. Order them correctly in the docs (although we could do that manually)

  2. Order them correctly in the Binder listing.

@sroet
Copy link
Collaborator Author

sroet commented Oct 23, 2020

I think README and examples should be enough.

Perfect, then this one is ready for a review

BTW, part of my upcoming examples refresh involves adding numbers to the notebooks. That will:

1. Order them correctly in the docs (although we could do that manually)

2. Order them correctly in the Binder listing.

Sounds great 😄

@sroet sroet changed the title WIP:add binder links Add binder links Oct 23, 2020
@sroet sroet requested a review from dwhswenson October 23, 2020 17:13
@sroet
Copy link
Collaborator Author

sroet commented Oct 23, 2020

@dwhswenson (just FYI for the possibilities of binder; the strangest binder setup I have done allows you to run Coupled-Cluster calculations in your browser Binder)

Copy link
Owner

@dwhswenson dwhswenson left a comment

Choose a reason for hiding this comment

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

Suggesting a little modification to move the Binder link higher up in the main examples page. Also, here's a better discussion of images/links/substitutions in RST than the one I linked in that comment:

Not necessarily for this PR, but nbsphinx has a trick for adding frontmatter that can automatically include a link to the relevant notebook on Binder:

(That link is also an example of this specific use in action.)

Consider that optional; if it is a pain to do, don't worry, but if it's easy, it would be nice. I don't mind having a Binder link for each page on RTD; I just didn't want to put Binder links in each ipynb, since it doesn't make sense if you've already loaded it locally.

@sroet
Copy link
Collaborator Author

sroet commented Oct 24, 2020

Consider that optional; if it is a pain to do, don't worry, but if it's easy, it would be nice. I don't mind having a Binder link for each page on RTD; I just didn't want to put Binder links in each ipynb, since it doesn't make sense if you've already loaded it locally.

So I got it working, but this only works for releases if the docs are build from a release install of contact_map (because it checks if the installed version of cantact map contains "dev")

[EDIT]
A bit of explanation of the version setting in the notebooks.

  1. If dev is part of the contact_map version we set the links to master (We assume that you want to build for latest in that case)
  2. else we set the version as v+version (I assumed our tags will always have that name)

@sroet sroet requested a review from dwhswenson October 24, 2020 13:29
@sroet
Copy link
Collaborator Author

sroet commented Oct 24, 2020

one thing that I could not figure out:
The link properly in docs/examples/index.rst can not be set depending on the version as that one does not seem to pass through jinja2 before converting to html

nvm, fixed it

@sroet
Copy link
Collaborator Author

sroet commented Oct 24, 2020

Alright, I also added jinja2 rendering for all the .rst, so you can do some logic in them as well. (All the jinja2 code in the .rst has access to all variables in the html_context dictionary.)

Comment on lines +69 to +70
html_context = {"release": release,
"version": version}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These will be the variables that are available to jinja2 code in the .rst

return
src = source[0]
rendered = app.builder.templates.render_string(
src, app.config.html_context
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this html_context is the dictionary build on line 69 and will provide the variables that jinja2 will put in scope during rendering.

Copy link
Owner

@dwhswenson dwhswenson left a comment

Choose a reason for hiding this comment

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

One very small change, then it looks good!

Co-authored-by: David W.H. Swenson <dwhs@hyperblazer.net>
@sroet
Copy link
Collaborator Author

sroet commented Oct 26, 2020

Thanks! Corrected that one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants