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 :only: option for code cells #629

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tuncbkose
Copy link

I made this primarily for myself but thought should make a PR if there would be any interest in merging. I'm happy to add tests/documentation, but I'd like to get an OK that the implementation is fine first.

Also, as far as I can tell, NbParserConfig.builder_name isn't used by anything, so I just co-opted it.

@bsipocz
Copy link
Collaborator

bsipocz commented Sep 19, 2024

I'm not sure yet if this will be widely used for notebooks, but cannot hurt the addition. Pending that tests and documentation will be added to this PR.

@bsipocz bsipocz added the enhancement New feature or request label Sep 19, 2024
@tuncbkose
Copy link
Author

While adding the test, I added a get_latex function to access the built file. For some reason though the built latex file is named python.tex instead of {filename}.tex. I could not figure out why, so if anyone has an idea I'd be interested to know.

@tuncbkose
Copy link
Author

There are two errors, 1 is a typo I made and the other is about that Tex file naming issue as far as I can tell. Will take a closer look later.

@agoose77
Copy link
Collaborator

agoose77 commented Sep 23, 2024

I haven't had a long think about this PR, but my initial sense is one of reluctance to merge it. The code-cell directive in MyST-NB is part of an (implicit) standard, and any new features should be something we carefully introduce. MyST-MD is an example of another project that implements the MyST spec.

I wonder here whether a post-process tool would be better suited, e.g. https://github.com/agoose77/sphinx-builder-classes. This extension allows you to add classes to your directives that are ignored for particular builders.

@tuncbkose
Copy link
Author

I understand if there may be good reasons not to merge this, but as far as I understand :load: is also not a part of the spec, and it has been sitting here for a while.

About the link, post-processing doesn't always help when you need the code outputs to be dependent on the builder. I appreciate that this is not a straightforward thing to implement in the best way, but maybe something flagged as experimental may be ok, following :load:s example. I'll be using it myself either way.

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.

3 participants