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

add the possibility to use custom hash function #1130

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

studyproject-mlds
Copy link

@studyproject-mlds studyproject-mlds commented Nov 12, 2020

Add the possibility to use custom hash function

joblib.Memory("cache_dir", hash_func=my_custom_hash_func)

@codecov
Copy link

codecov bot commented Nov 12, 2020

Codecov Report

Merging #1130 (a7037e3) into master (53a8173) will increase coverage by 0.37%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1130      +/-   ##
==========================================
+ Coverage   93.93%   94.30%   +0.37%     
==========================================
  Files          47       47              
  Lines        6956     6958       +2     
==========================================
+ Hits         6534     6562      +28     
+ Misses        422      396      -26     
Impacted Files Coverage Δ
joblib/memory.py 95.58% <85.71%> (+0.02%) ⬆️
joblib/test/test_parallel.py 95.62% <0.00%> (-1.31%) ⬇️
joblib/test/test_memory.py 98.56% <0.00%> (+0.14%) ⬆️
joblib/pool.py 87.80% <0.00%> (+0.81%) ⬆️
joblib/func_inspect.py 92.26% <0.00%> (+1.19%) ⬆️
joblib/logger.py 86.84% <0.00%> (+1.31%) ⬆️
joblib/disk.py 92.06% <0.00%> (+1.58%) ⬆️
joblib/test/test_memmapping.py 99.23% <0.00%> (+1.90%) ⬆️
joblib/_memmapping_reducer.py 96.60% <0.00%> (+2.26%) ⬆️
joblib/backports.py 84.21% <0.00%> (+39.47%) ⬆️

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 53a8173...a7037e3. Read the comment docs.

Copy link
Contributor

@tomMoral tomMoral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few formatting comments.

The change seems quite small so I would say it is okay but could you precise the use case you see for this change?

Most of the when I need to change the hashing, it is for some custom objects and I implement a custom __reduce__ which is sufficient to change the hash behavior. I am curious of use cases that would not be handled by this.

joblib/memory.py Outdated Show resolved Hide resolved
joblib/memory.py Outdated Show resolved Hide resolved
joblib/memory.py Outdated Show resolved Hide resolved
joblib/memory.py Outdated Show resolved Hide resolved
joblib/memory.py Outdated Show resolved Hide resolved
@studyproject-mlds
Copy link
Author

studyproject-mlds commented Nov 12, 2020

Just a few formatting comments.

The change seems quite small so I would say it is okay but could you precise the use case you see for this change?

Most of the when I need to change the hashing, it is for some custom objects and I implement a custom __reduce__ which is sufficient to change the hash behavior. I am curious of use cases that would not be handled by this.

I have a lot of custom objects and I can't implement __reduce__ for all of them.

@studyproject-mlds studyproject-mlds force-pushed the add-custom-hash-func branch 2 times, most recently from f63c376 to 404c41d Compare November 12, 2020 18:12
studyproject-mlds and others added 2 commits November 12, 2020 19:22
Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>
@judahrand
Copy link

I have an alternate implementation in #1232 which I believe adheres more closely to how other custom 'plugin' type functionality works in Joblib. Examples being store backends and compressors.

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