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

bpo-35784: Include optional kwargs in hashlib.new() documentation #15890

Open
wants to merge 2 commits into
base: master
from

Conversation

@tahia-khan
Copy link
Contributor

commented Sep 10, 2019

Updating documentation for hashlib.new(), including Doc/library/hashlib.rst and the docstring in Lib/hashlib.py.

https://bugs.python.org/issue35784

https://bugs.python.org/issue35784

@gpshead
Copy link
Member

left a comment

A news entry isn't really necessary for a documentation update, but is harmless regardless. :)

@rhettinger

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2019

additional keyword arguments that are specific to the selected hashing algorithm

This is going to raise the question of which keywords are available for each algorithm and what they do. It would be nice if we had a summary table (like that at the top of the itertools() documentation).

@tiran tiran added the DO-NOT-MERGE label Sep 11, 2019

@tiran

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

While you are technically correct, I would like to hold off on this change for a bit more. I plan to replace our internal vendored copies of sha3 and blake2 with OpenSSL code. OpenSSL 1.1.1 has some of the new required APIs. OpenSSL 3.0.0-dev is currently adding more but with slightly different semantic. For example OpenSSL does not have blake2b, but blake2b512 for hashing and blake2bmac for keyed hashing with salting and personalization.

I like to get integration with OpenSSL right until hashlib.new() is officially documented.

@gpshead

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

This is going to raise the question of which keywords are available for each algorithm and what they do. It would be nice if we had a summary table (like that at the top of the itertools() documentation).

Lets not make such a table. People shouldn't use hashlib.new() in most code. It is a conveinence function for the use case where someone needs to choose a hash algorithm at runtime. I don't want to duplicate documentation for the same thing in two places in hashlib.rst.

The one true canonical place for arguments for any given hash function to be defined is in the documentation for the hashlib.$algorithmname() constructor. Or individual documentation specific to each algorithm should we wind up in a situation where a $algorithmname constructor no longer makes sense (as some of the plethora of suffixed blake$mumble variants might do). Not as part of hashlib.new().

@gpshead gpshead assigned tiran and unassigned gpshead Sep 11, 2019

@tahia-khan

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2019

@tiran should this PR be closed then, since the semantic might change anyway after you finish integrating OpenSSL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.