bpo-30174: remove duplicate definition from pickletools#1301
Conversation
There's another almost identical definition of bytes1 above this one. The only difference is a minor one in the docstring: the first one has "the number of bytes in the string" and the second one has just "the number of bytes". I like the first wording better so I kept it.
|
@JelleZijlstra, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tim-one, @mdickinson and @avassalotti to be potential reviewers. |
|
I think this qualifies as "trivial" but I don't have the rights to add that label. |
|
Unfortunately, this is a code cleanup so we at least need to ping pickle maintainers on bugs.p.o. Sometimes there might be a valid or historical reason for this kind of code duplication. |
|
OK, I'll open an issue. |
|
I'm the original pickletools author, and the change looks good to me. The "bytes" type didn't exist at the time, so this code was added later. The current duplication obviously serves no purpose, and was certainly a mistake. I don't understand how the new git workflow works yet else I'd do something tangible to get this patch applied ;-) |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
The variant without " in the string" is more correct and consistent with bytes4 and bytes8.
There's another almost identical definition of bytes1 above this one.
The only difference is a minor one in the docstring: the first one has
"the number of bytes in the string" and the second one has just "the
number of bytes". I like the first wording better so I kept it.