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

Handle new naming scheme in newer Jupyter notebook #1214

Merged
merged 3 commits into from
Sep 13, 2021

Conversation

mjziebarth
Copy link
Contributor

As mentioned in #1210, this adds handling of a different cell code naming scheme in the newest Jupyter notebook / IPython version (at least as packaged on Arch Linux).

Let me know if anything else can / needs to be done.

Cheers!

…sion breaking the across-run identification of annotations in notebooks.
@codecov
Copy link

codecov bot commented Sep 3, 2021

Codecov Report

Merging #1214 (c6f7a29) into master (0fa2cb9) will decrease coverage by 5.82%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1214      +/-   ##
==========================================
- Coverage   94.51%   88.68%   -5.83%     
==========================================
  Files          47       47              
  Lines        7034     7052      +18     
==========================================
- Hits         6648     6254     -394     
- Misses        386      798     +412     
Impacted Files Coverage Δ
joblib/func_inspect.py 92.34% <50.00%> (-0.47%) ⬇️
joblib/test/test_dask.py 3.73% <0.00%> (-95.15%) ⬇️
joblib/_dask.py 24.13% <0.00%> (-69.96%) ⬇️
joblib/hashing.py 89.47% <0.00%> (-1.76%) ⬇️
joblib/test/test_parallel.py 95.76% <0.00%> (-1.34%) ⬇️
joblib/memory.py 95.37% <0.00%> (-0.19%) ⬇️
joblib/test/test_memory.py 98.62% <0.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fa2cb9...c6f7a29. Read the comment docs.

@ogrisel
Copy link
Contributor

ogrisel commented Sep 13, 2021

Thanks @mjziebarth, I pushed a small improvement to your PR. I tested the fix locally and I confirm that it works as expected. There is no easy way to write a test for this so... so be it.

BTW next time, please create your PR from a dedicated branch instead of using master. Otherwise it will be very confusing for your to sync your local joblib repo with the main joblib repo.

@ogrisel ogrisel merged commit 0730b77 into joblib:master Sep 13, 2021
@ogrisel
Copy link
Contributor

ogrisel commented Sep 13, 2021

Thanks @mjziebarth!

@ianhi ianhi mentioned this pull request Oct 1, 2021
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 7, 2021
Release 1.1.0
Fix byte order inconsistency issue during deserialization using joblib.load in cross-endian environment: the numpy arrays are now always loaded to use the system byte order, independently of the byte order of the system that serialized the pickle. joblib/joblib#1181
Fix joblib.Memory bug with the ignore parameter when the cached function is a decorated function. joblib/joblib#1165
Fix joblib.Memory to properly handle caching for functions defined interactively in a IPython session or in Jupyter notebook cell. joblib/joblib#1214
Update vendored loky (from version 2.9 to 3.0) and cloudpickle (from version 1.6 to 2.0) joblib/joblib#1218
jjerphan pushed a commit to jjerphan/joblib that referenced this pull request Oct 11, 2021

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 14, 2021
Release 1.1.0
Fix byte order inconsistency issue during deserialization using joblib.load in cross-endian environment: the numpy arrays are now always loaded to use the system byte order, independently of the byte order of the system that serialized the pickle. joblib/joblib#1181
Fix joblib.Memory bug with the ignore parameter when the cached function is a decorated function. joblib/joblib#1165
Fix joblib.Memory to properly handle caching for functions defined interactively in a IPython session or in Jupyter notebook cell. joblib/joblib#1214
Update vendored loky (from version 2.9 to 3.0) and cloudpickle (from version 1.6 to 2.0) joblib/joblib#1218
@timcoote
Copy link

Just checking..... I upgraded to 1.1.0 and found that this invalidated caches created by the earlier version of the module used within a Jupyter notebook. check_call_in_cache fails to find the earlier versions of the function, which, presumably were in different filenames.

Is this as expected: should earlier cached function/args/kwargs sets fail? I'd hoped that this wasn't the case, but I can understand if it is.

@ogrisel
Copy link
Contributor

ogrisel commented Oct 28, 2021

I believe this is expected but what I would not expect is that the cache would have worked without invalidation using a recent version of jupyter and the older version of joblib.

@NikolayXHD
Copy link

NikolayXHD commented Jun 14, 2022

Hi, the fix did not work for me, IPython.__version__ '8.4.0'

parts array in func_inspect.py#L151 looks like this:

['', 'tmp', 'ipykernel_25910', '649976777.py']

UPDATE: the Memory cache IS preserved between IPykernel sessions, however, when the cell with defined function is updated, the last part '649976777.py' is changed, effectively invalidating the cache.

I believe joblib.Memory is supposed to invalidate the cache whenever the function code changes, using joblib.func_inspect.get_func_code to detect code change. If so, then variation in last path segment '649976777.py' should be ignored, since it does not imply the change in cached function code.

def get_func_name():
    # ...
    parts[-2] = 'ipykernel'
    # add this line to prevent invalidating cahe on arbitrary change
    # in jupyter cell code
    del parts[-1]

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

4 participants