-
Notifications
You must be signed in to change notification settings - Fork 10
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 EnsureHashAlgorithm #492
Conversation
This adds EnsureHashAlgorithm constraint which checks whether the given algorith name is contained in hashlib's algorithms_guaranteed set. It derives from EnsureChoice without further customization. Using `algorithms_guaranteed` covers algorithms that have named constructors in hashlib, i.e. accessible through getattr(hashlib, name) - that's the current usage in MultiHash. Future expansion could see us move to algorithms_available - these will be recognized when passed to (slower) hashlib.new(), but have no named constructors. For details, see: https://docs.python.org/3.11/library/hashlib.html
The EnsureHash algorithm now calls its super.__init__ correctly. We use EnsureHashAlgorithm() | EnsureListOf(...) in ls_file_colelction now. This correctly handles incoming values, which are always a list in CLI (argparse behaviour with --append) and can be str or list in Python API. At least when they are OK. When input is not OK, two things are suboptimal: - an incorrect hash (str) proceeds to EnsureListOf, which tries to go letter by letter, failing on first - a hash (list) with an incorrect value would first fail on EnsureHashAlgorithm(list), proceed to EnsureListOf, and fail on EnsureHashAlgorithm(str) As a conequence, in both cases the error message would say "does not match any of 2 alternatives" and print EnsureChoice message ("is not one of ...") twice.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #492 +/- ##
==========================================
+ Coverage 92.33% 92.39% +0.05%
==========================================
Files 125 125
Lines 9530 9549 +19
Branches 1025 1028 +3
==========================================
+ Hits 8800 8823 +23
+ Misses 708 704 -4
Partials 22 22
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviving this feature! I added a changelog, minimal documentation, and a missing test to reach 100% coverage. It tested it, and it LGTM!
This PR revives #357, which I couldn't push into directly. I didn't add much to it, I committed a patch @mih proposed a while ago already on top of it, and I added a basic set of tests for the constraint.