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

Fix - Do not split abspath with dot #1364

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

Conversation

VaelK
Copy link

@VaelK VaelK commented Nov 23, 2022

Hello,

Following issue #1362, this improvement avoid splitting the absolute path whenever a dot is found in it. Let's say that a folder in this path is named name.folder, then the test test_parallel_call_cached_function_defined_in_jupyter does not pass. Plus it create an extra level in the tree structure of backend store.

Let me know if there is a problem with that.

Thank you!

@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Base: 93.96% // Head: 93.95% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (0de702a) compared to base (796e7aa).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1364      +/-   ##
==========================================
- Coverage   93.96%   93.95%   -0.02%     
==========================================
  Files          52       52              
  Lines        7291     7291              
==========================================
- Hits         6851     6850       -1     
- Misses        440      441       +1     
Impacted Files Coverage Δ
joblib/func_inspect.py 92.34% <100.00%> (ø)
joblib/test/test_store_backends.py 91.42% <0.00%> (-5.72%) ⬇️
joblib/pool.py 87.80% <0.00%> (-0.82%) ⬇️
joblib/parallel.py 96.03% <0.00%> (-0.53%) ⬇️
joblib/_parallel_backends.py 93.01% <0.00%> (ø)
joblib/test/test_parallel.py 97.05% <0.00%> (ø)
joblib/memory.py 95.51% <0.00%> (+0.26%) ⬆️
joblib/test/testutils.py 100.00% <0.00%> (+50.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@VaelK
Copy link
Author

VaelK commented Nov 24, 2022

The doc build failed because of an URL not reached:
urllib.error.URLError: <urlopen error [Errno 104] Connection reset by peer>
It is not related to my work. Rerunning the workflow should fix this.

@VaelK
Copy link
Author

VaelK commented Nov 26, 2022

Ok i think that some generated code in readthedocs tries to load a csv from an extrernal source (url) but fails since the server send a 104 error... I have no idea how to track this since I don't know where the code is generated. If one has any clue about that...

@ogrisel
Copy link
Contributor

ogrisel commented Feb 15, 2023

Could you please try to craft a minimal non-regression test for this fix?

@ogrisel
Copy link
Contributor

ogrisel commented Feb 15, 2023

Let's ignore the readthedocs failure for now. If it still happens let's fix it in an dedicated PR and keep this PR focused on the original issue.

@VaelK
Copy link
Author

VaelK commented Feb 15, 2023

Ok, fine for me.
Thanks!

@jjerphan
Copy link
Contributor

To complete @ogrisel's remark in #1364 (comment) here is scikit-learn's section on crafting minimal reproducer.

A non-regression test can then be added in this PR using this reproducer.

@VaelK
Copy link
Author

VaelK commented Mar 4, 2023

Here is a minimal reproducer using shell command on linux with anaconda for setting the virtual environnement:

conda create --name test_joblib_issue_1364 python=3.9
conda activate test_joblib_issue_1364
pip install pytest==7.2.2
pip install gitpython==3.1.31
mkdir joblib.issue.1364
cd joblib.issue.1364/
git clone https://github.com/joblib/joblib.git
cd joblib
pytest joblib/test/test_memory.py
cd ../..
rm -rf joblib.issue.1364
conda remove --name test_joblib_issue_1364 --all

Expected result:
============================= 52 passed, 2 skipped, 4 warnings in 4.10s ====================================

Actual result:
====================================== short test summary info =========================================
FAILED joblib/test/test_memory.py::test_parallel_call_cached_function_defined_in_jupyter[True] - AssertionError: assert 'ipython-input' in 'main--home-baba-projets_git-joblib'
FAILED joblib/test/test_memory.py::test_parallel_call_cached_function_defined_in_jupyter[False] - AssertionError: assert 'ipython-input' in 'main--home-baba-projets_git-joblib'
=========================== 2 failed, 50 passed, 2 skipped, 4 warnings in 4.48s ================================

I am crafting a non regression test now.

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

3 participants