Skip to content

Conversation

@olivialynn
Copy link
Member

@olivialynn olivialynn commented Mar 13, 2023

Adding a rendered notebooks option to the template.

Highlights include:

  • moving the nb/ directory into 'docs/' (I found it the best way for the .rst referencing to remain intuitive--I don't think this loses too much over having it sit next to src/, but if anyone has a better method I'm all ears)
  • converting requirements.txt into a jinja template to optionally include notebook dependencies
  • adding jinja whitespace specifiers to remove awkward spacing around conditionally-included text in toml, conf, etc.

Will be followed shortly with some writing in the RTD documentation on notebooks. I'd also like to add conditioning to the CI (left an if: ${{ true }} placeholder for now) to only install pandoc and other notebook requirements if option was selected, but this should be done in tandem with other CI changes I'm opening an issue for.

@olivialynn olivialynn marked this pull request as ready for review March 13, 2023 15:43
@olivialynn olivialynn changed the title Issue/34/notebooks Adding notebook support Mar 13, 2023
"sphinx_rtd_theme==1.2.0", # Used to render documentation
"sphinx-autoapi==2.0.1", # Used to automatically generate api documentation
{% if preferred_linter == 'pylint' %}
{%- if preferred_linter == 'pylint' %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, that's nice. Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

+100

@drewoldag
Copy link
Collaborator

One slight change that I think is worth making here - I renamed the nb directory to notebooks in PR #83, so if you have a chance to make that change, that would be nice :)

Also, I'm curious if you've had a chance to confirm that the pre-commit hook that will clear output from jupyter notebooks will pass the test immediately after hydrating a project from the template? I was running into an issue early on when I had a demo notebook where the pre-commit hook would find some small thing that had changed, and it would result in a failed pre-commit hook test for the very first commit.

Copy link
Collaborator

@drewoldag drewoldag left a comment

Choose a reason for hiding this comment

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

This looks awesome. Thanks for putting it together!

"sphinx_rtd_theme==1.2.0", # Used to render documentation
"sphinx-autoapi==2.0.1", # Used to automatically generate api documentation
{% if preferred_linter == 'pylint' %}
{%- if preferred_linter == 'pylint' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

+100

@olivialynn
Copy link
Member Author

One slight change that I think is worth making here - I renamed the nb directory to notebooks in PR #83, so if you have a chance to make that change, that would be nice :)

Nice catch, thank you!

Also, I'm curious if you've had a chance to confirm that the pre-commit hook that will clear output from jupyter notebooks will pass the test immediately after hydrating a project from the template? I was running into an issue early on when I had a demo notebook where the pre-commit hook would find some small thing that had changed, and it would result in a failed pre-commit hook test for the very first commit.

I haven't, but I'll admit I now default to editing notebooks in plaintext if possible with how messy things get in RAIL. I'll take a look once I get this in.

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.

4 participants