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

Rework parsing logic inside nbdev_sidebar & Auto-add index file onto section #1307

Merged
merged 2 commits into from Oct 19, 2023

Conversation

p4perf4ce
Copy link
Contributor

Why?

#1300 found an issue on nbdev_sidebar being unable to handle multi-level project structure properly (in a way that you think it should do).

Summary

This PR should now handle multi-level structure project correctly.
Additionally, nbdev_sidebar now auto-detect the index file and added them to section.

TL;DR:

  • For sidebar, multi-level project structure is now rendered as it is structured.
  • For sidebar, user no longer have to add href:/path/to/index manually if there is an index file in the directory.

Proposed change

  • Instead of writing yaml directly. files were parsed into a directory structure as a dict then converted into a quarto format by _recursive_parser and dumped to yaml using yaml.dump.
  • IndentDumper class were added to correct weird PyYAML indentation style that is not consistent with previous version of sidebar.yaml.
  • Instead of adding href manually, any proper index\.* will be added to the section as href: /path/to/index automatically. This behavior is controlled by _recursive_parser(..., set_index: bool = True).

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@p4perf4ce p4perf4ce changed the title Rework parsing logic inside nbdev_sidebar & Auto-add index.* onto section Rework parsing logic inside nbdev_sidebar & Auto-add index file onto section Mar 11, 2023
nbs/api/14_quarto.ipynb Show resolved Hide resolved
nbs/api/14_quarto.ipynb Show resolved Hide resolved
@p4perf4ce
Copy link
Contributor Author

Sorry for the very late reply @nesaboz, I've put your suggest change into the PR. But I doubt that this PR will be merged since this repo looks abandoned 😅.

@jph00
Copy link
Member

jph00 commented Oct 19, 2023

Many thanks - sorry this took so long to merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants