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

New MultiHash class #345

Merged
merged 1 commit into from
May 9, 2023
Merged

New MultiHash class #345

merged 1 commit into from
May 9, 2023

Conversation

mih
Copy link
Member

@mih mih commented May 8, 2023

This has factored been factored out from the UrlOperations classes. It simplifies that code and can also be used elsewhere.

Closes #344

This has been factored out from the `UrlOperations` classes. It
simplifies that code and can also be used elsewhere.

Closes datalad#344
@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Patch coverage: 95.91% and project coverage change: +0.06 🎉

Comparison is base (9e0d341) 91.92% compared to head (96bea83) 91.98%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
+ Coverage   91.92%   91.98%   +0.06%     
==========================================
  Files          84       86       +2     
  Lines        7455     7478      +23     
==========================================
+ Hits         6853     6879      +26     
+ Misses        602      599       -3     
Impacted Files Coverage Δ
datalad_next/url_operations/ssh.py 53.90% <33.33%> (+0.75%) ⬆️
datalad_next/url_operations/__init__.py 82.35% <100.00%> (-1.40%) ⬇️
datalad_next/url_operations/file.py 91.83% <100.00%> (-0.09%) ⬇️
datalad_next/url_operations/http.py 94.38% <100.00%> (-0.07%) ⬇️
datalad_next/utils/multihash.py 100.00% <100.00%> (ø)
datalad_next/utils/tests/test_multihash.py 100.00% <100.00%> (ø)

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

@mih mih merged commit 9c65b96 into datalad:main May 9, 2023
@mih mih deleted the multihash branch May 9, 2023 04:37
@yarikoptic
Copy link
Member

yarikoptic commented May 9, 2023

FWIW name MultiHash might be a bit suboptimal since collides with a known https://github.com/multiformats/multihash .

Note that in datalad core we have Digester for AFAIK pretty much the same functionality: https://github.com/datalad/datalad/blob/HEAD/datalad/support/digests.py#L25

@mih
Copy link
Member Author

mih commented May 10, 2023

Yes, I am aware of Digester, but it focuses on file-likes, hence it could not be used for the UrlOperations. This PR only took that original implementation and generalized and moved it into its own piece, because #343 also needs this.

Thanks for pointing out the name clash. I was not aware of this. Right now I think that a utility class conflicting with a package is not optimal indeed, but also not a major problem.

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.

Factor out multi-hash helper from UrlOperations
2 participants