ENH add auto .gitignore in the Memory cache folder by StefanieSenger · Pull Request #1674 · joblib/joblib

StefanieSenger · 2025-02-27T12:04:19Z

This PR adds an automatically created .gitignore file to the cache folder created with Memory().__init__().

~~Memory().clear() and rm_subdirs() are modified to delete joblib's .gitignore file while removing the other joblib folders.~~` Now it stays.

PR also add a corresponding test.

joblib/__init__.py

joblib/_store_backends.py

tomMoral

For directory that we do not create, I would not add the .gitignore.
I also added a few nipicks.

joblib/_store_backends.py

joblib/disk.py

joblib/test/test_memory.py

codecov · 2025-02-27T13:08:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.43%. Comparing base (389daf2) to head (5699336).
Report is 25 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1674      +/-   ##
==========================================
- Coverage   95.51%   95.43%   -0.08%     
==========================================
  Files          46       46              
  Lines        7711     7731      +20     
==========================================
+ Hits         7365     7378      +13     
- Misses        346      353       +7

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…them

StefanieSenger · 2025-02-28T09:44:50Z

joblib/disk.py

-    The directory indicated by `path` is left in place, and its subdirectories
-    are erased.
+    The directory indicated by `path` is left in place, and its subdirectories and the
+    .gitignore file are erased.


An empty directory is not tracked by git anyways, but I assume there must be some benefit of re-using this directory somehow (so the .gitignore file should also be removed), otherwise I cannot see how users would benefit from a function like this one - compared to simply deleting the whole folder.

Let's say users have added other files into the cache folder - they would be wiped as well. This is what puzzles me the most about this...

Good catch but I would not add .gitignore logic here, mostly for simplicity reasons.

rm_subdirs seems to imply that it deletes subdirectories so it was not deleting any file before and I think we can decide to keep it like this, at least not changing anything doesn't seem to have any downside

Thank you, @lesteve, I have removed this. So the file stays in the new folder also after running rm_subdirs() or clearing cache.

joblib/test/test_memory.py

StefanieSenger

Hello @lesteve and @tomMoral,

thank you for your comments, I have fixed the doctest failure by adding a verbosity level to the memory object in this example (still not sure why this didn't raise before, since it was unrelated to the PR) and have implemented your requested changes.

This PR is ready for a review.

StefanieSenger · 2025-03-03T08:50:46Z

joblib/disk.py

-    The directory indicated by `path` is left in place, and its subdirectories
-    are erased.
+    The directory indicated by `path` is left in place, and its subdirectories and the
+    .gitignore file are erased.


Thank you, @lesteve, I have removed this. So the file stays in the new folder also after running rm_subdirs() or clearing cache.

joblib/test/test_memory.py

ogrisel

I reopened a tracking issue for the str vs pathlib.Path inconsistency.

+1 for merging this PR without resolving that bug and instead fix it in a dedicated PR.

joblib/test/test_memory.py

ogrisel · 2025-03-03T15:29:54Z

joblib/_store_backends.py

+        # automatically add `.gitignore` file to the cache folder; the condition is
+        # necessary because in Memory.__init__, the user passed `location` param is
+        # modified to be either `{location}` or `{location}/joblib `depending on input
+        # type


Suggested change

# automatically add `.gitignore` file to the cache folder; the condition is

# necessary because in Memory.__init__, the user passed `location` param is

# modified to be either `{location}` or `{location}/joblib `depending on input

# type

# Automatically add `.gitignore` file to the cache folder.

# XXX: the condition is necessary because in `Memory.__init__`, the user

# passed `location` param is modified to be either `{location}` or

# `{location}/joblib `depending on input type (`pathlib.Path` vs `str`).

# The proper resolution of this inconsistency is tracked in:

# https://github.com/joblib/joblib/issues/1684

I commited this, but what is XXX supposed to mean or stand for?

ogrisel · 2025-03-03T15:33:28Z

@StefanieSenger can you please add an entry in the changelog to document this enhancement?

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

tomMoral

A few more comments but I think we are close to the end

joblib/_store_backends.py

joblib/test/test_memory.py

Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>

StefanieSenger · 2025-03-05T08:47:18Z

Thank you for reviewing and taking care of absolute paths, @tomMoral. I had not been aware that you could pass them.

I have committed your suggestions.

tomMoral · 2025-03-05T09:44:03Z

Merging!
Thanks a lot @StefanieSenger for pushing this through! 🎉
It will avoid a lot of cache to end up in github repo ^^

ENH add auto .gitignore in the Memory cache folder

a71c3b0

StefanieSenger commented Feb 27, 2025

View reviewed changes

joblib/__init__.py Outdated Show resolved Hide resolved

satisfy black

4b90ea0

StefanieSenger commented Feb 27, 2025

View reviewed changes

joblib/_store_backends.py Show resolved Hide resolved

StefanieSenger and others added 2 commits February 27, 2025 13:30

Merge branch 'main' into gitignore

7982400

satisfy ruff

6cf5bd5

tomMoral reviewed Feb 27, 2025

View reviewed changes

StefanieSenger added 2 commits February 27, 2025 13:56

empty commit to allow pre-commit to work

f16d037

satisfy ruff-format too

eac852d

StefanieSenger added 5 commits February 27, 2025 14:31

remove cache file also if test fails

3b85c40

apply suggestions from code review

52424c4

add condition on only deleting gitignore files if joblib has created …

526c48c

…them

add test for correctly removing .gitignore

f5e88fc

add comment; refactor test

828c4fb

StefanieSenger commented Feb 28, 2025

View reviewed changes

StefanieSenger added 2 commits February 28, 2025 11:05

Merge remote-tracking branch 'upstream/main' into gitignore

f3cc83f

fix doctest and ruff

a98f15e

lesteve reviewed Feb 28, 2025

View reviewed changes

joblib/test/test_memory.py Outdated Show resolved Hide resolved

StefanieSenger added 7 commits February 28, 2025 14:03

set Memory(verbose=0) to get consistent result

796f131

skip test if numpy not installed

e3bb551

change verbosity in example to 1

483856b

remove code that removes .gitignore

9ad2e22

check the full content of .gitignore in test

d84e087

use @with_numpy instead of @pytest.mark.skipif

c3cff4b

read file with context manager

86b9ec6

StefanieSenger commented Mar 3, 2025

View reviewed changes

ogrisel approved these changes Mar 3, 2025

View reviewed changes

Apply suggestions from code review

4296001

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

StefanieSenger and others added 3 commits March 4, 2025 08:58

add changelog entry

d1f22ea

Merge branch 'main' into gitignore

3da8b26

satisfy ruff

7328463

tomMoral approved these changes Mar 4, 2025

View reviewed changes

joblib/_store_backends.py Outdated Show resolved Hide resolved

joblib/_store_backends.py Show resolved Hide resolved

joblib/test/test_memory.py Outdated Show resolved Hide resolved

StefanieSenger and others added 3 commits March 5, 2025 09:31

Apply suggestions from code review

11cfa20

Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>

Update joblib/_store_backends.py

832c324

Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>

satisfy ruff

5699336

tomMoral merged commit adc3a7c into joblib:main Mar 5, 2025
29 checks passed

StefanieSenger deleted the gitignore branch March 5, 2025 12:12

-        # automatically add `.gitignore` file to the cache folder; the condition is
-        # necessary because in Memory.__init__, the user passed `location` param is
-        # modified to be either `{location}` or `{location}/joblib `depending on input
-        # type
+        # Automatically add `.gitignore` file to the cache folder.
+        # XXX: the condition is necessary because in `Memory.__init__`, the user
+        # passed `location` param is modified to be either `{location}` or
+        # `{location}/joblib `depending on input type (`pathlib.Path` vs `str`).
+        # The proper resolution of this inconsistency is tracked in:
+        # https://github.com/joblib/joblib/issues/1684

Conversation

StefanieSenger commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomMoral left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

StefanieSenger Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lesteve Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

StefanieSenger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ogrisel Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

ogrisel commented Mar 3, 2025

Uh oh!

tomMoral left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StefanieSenger commented Mar 5, 2025

Uh oh!

Uh oh!

tomMoral commented Mar 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

StefanieSenger commented Feb 27, 2025 •

edited

Loading

codecov bot commented Feb 27, 2025 •

edited

Loading

StefanieSenger Feb 28, 2025 •

edited

Loading

lesteve Feb 28, 2025 •

edited

Loading

StefanieSenger left a comment •

edited

Loading