-
Notifications
You must be signed in to change notification settings - Fork 0
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
updating cacheing code #1
updating cacheing code #1
Conversation
|
||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this try try .. except
block be inside the else
statement above https://github.com/AakashGfude/MyST-NB/pull/1/files#diff-e2066ea776dc9a4cfa8977efdc1df7abR41?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I think it could make sense to combine them somehow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. I will merge the PR then. After that, we can play around. Thank you for the code.
# Populate the outputs either with nbclient or a cache | ||
path_cache = document.settings.env.config['jupyter_cache'] | ||
if path_cache is True: | ||
path_cache = Path(document.settings.env.srcdir).joinpath('.jupyter_cache') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @choldgraf , does it make more sense to have the .jupyter_cache
folder inside the source directory? My intuition was to have it at the same level as build
and source
folders, top level of the project folder.
Or maybe, when I think more about it, inside the build
directory as sphinx does for its doctree cache? Then make clean
will also clear the cache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, do we think that people will want "make clean" to clear the cache? If so, that could be a good idea to put it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but it also might be my familiarity bias speaking. Because we have the same thing in jupinx for clearing execution cache, following the sphinx philosophy of storing doctree cache in build.
Basically we have specific cleanses like make clean-html
, make clean-execute
.For a clean slate, we have make clean
.
One other advantage I think can be distribution. The build
folder as a whole can be distributed, which has outputs and cache both in it?
Hey @AakashGfude - I had started a PR of my own to include jupyter cacheing. This PR to your branch has a few suggested improvements that I ported over from my PR.
Feel free to modify / not accept some things, or to just merge and then make modifications as you wish. I just wanted to offer the PR so we didn't duplicate effort as much. I'll move my efforts over to other stuff while you finish up the cacheing PR