-
Notifications
You must be signed in to change notification settings - Fork 192
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
Docs: move in the documentation on caching #4228
Conversation
cf8b9fe
to
a91a35c
Compare
docs/source/howto/run_codes.rst
Outdated
|
||
`#3988`_ | ||
Finally, while caching saves unnecessary computations, it does not save disk space: The output nodes of the cached calculation are full copies of the original outputs. |
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.
Is there any reason that this is shown as separate paragraph instead of just another bullet point?
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.
Don't think so. Will include as bullet point
The content is mostly moved in from existing content, just adapting the style to the new guidelines.
Split off the technical details to a Topics section. I put it under the "Provenance" topic, as I don't think it needs its own top level section. Even though caching only applies to calculation jobs at the moment, and so one could argue to place it there, really this is a current implementation detail and this is explained in the limitations. Since it has fundamentally to do with the provenance, I think this is the best fit. Finally, I put an `important` block at the beginning of the how-to to explain why it is not enabled by default and to warn users of the caveats, which links to the topics section.
a91a35c
to
1ce8f94
Compare
Thanks for the review @csadorf . I have addressed the simple comments and split off the details to a Topics section, and added a disclaimer. See commit message for reasoning of location of new section. |
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.
Good to go!
Fixes #3988
The content is mostly moved in from existing content, just adapting the
style to the new guidelines.