gh-143103: Added pad parameter to base64.z85encode
#143106
Conversation
Misc/NEWS.d/next/Library/2025-12-23-17-07-22.gh-issue-143103.LRjXEW.rst
Outdated
Show resolved
Hide resolved
haukex
changed the title
pad argument to base64.z85encodepad parameter to base64.z85encode
|
All suggested changes applied including correcting "argument" to "parameter" in docs and commit message as well; the force push was just a squash of those changes. |
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
I am reviewing on my phone so check my suggestions before committing them (like typos or things like that).
Misc/NEWS.d/next/Library/2025-12-23-17-07-22.gh-issue-143103.LRjXEW.rst
Outdated
Show resolved
Hide resolved
This makes it analogous to `a85encode` and `b85encode` and allows the user to more easily meet the Z85 specification, which requires input lengths to be a multiple of 4.
|
@picnixz Thanks, I applied the changes and force-pushed the squashed result. |
|
Thanks. In the future do not force push. We squash everything at the end so it doesn't help (it however gets harder for incremental reviews). Force-pushing is reserved when the history got messed up (e.g. bad merge) |
|
Looks good but can you add a What's New entry as well for this change? just restate what you did in the NEWS entry (look at Doc/whatsnew/3.15.rst to see what to do). TiA |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
LGTM. 👍
Side note. I thought about proposing to make pad a keyword-only parameter. Boolean arguments almost always should be passed by keyword, otherwise it is errorprone if you pass many arguments. This is not applied when the number of parameters is small (like in str.splitlines()), and this may be one of such cases. So I am not sure.
|
AFAIK, not all minor changes deserve an entry in What's New. |
|
I also wanted to suggest a kw-only but b85* functions do not have a kw-only and I think it is better to keep similar signatures for the encoding functions. |
Yeah but I think it is nice to have features there. Otherwise they get burried in the bugfixes as well |
3 participants
This makes
z85encodeanalogous toa85encodeandb85encodeand allows the user to more easily meet the Z85 specification, which requires input lengths to be a multiple of 4. For details see #143103.Documentation and tests included. Tests were verified against the Z85 reference implementation (which I wrapped into a Python module at haukex/py-z85).
📚 Documentation preview 📚: https://cpython-previews--143106.org.readthedocs.build/