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

feat: avoid polluting memory namespace when caching functions in jupyter notebooks #1570

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

banteg
Copy link

@banteg banteg commented Apr 9, 2024

while joblib previously correctly identified that part of the dynamically generated folder name for functions defined in a notebook depended on the pid and should be cut, it has missed that the filename was dynamically generated too and depended on the murmur2 hash of the function code.

this has lead to pollution of memory namespace on each function change. instead, we collapse the ipykernel namespace to a single static part and let joblib.Memory handle the invalidation on code changes as it's supposed to do.

you can learn more here https://github.com/ipython/ipykernel/blob/main/ipykernel/compiler.py#L90

…ter notebooks

while joblib previously correctly identified that part of the dynamically generated folder name for functions defined in a notebook depended on the pid and should be cut, it has missed that the filename was dynamically generated too and depended on the murmur2 hash of the function code.

this has lead to pollution of memory namespace on each function change. instead, we collapse the ipykernel namespace to a single static part and let joblib.Memory handle the invalidation on code changes as it's supposed to do.
Copy link

codecov bot commented Apr 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 95.13%. Comparing base (1b0d1f4) to head (14a7905).

Files Patch % Lines
joblib/func_inspect.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1570      +/-   ##
==========================================
- Coverage   95.16%   95.13%   -0.03%     
==========================================
  Files          45       45              
  Lines        7674     7674              
==========================================
- Hits         7303     7301       -2     
- Misses        371      373       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

None yet

1 participant