[issue27506] make bytes/bytearray delete a keyword argument

Martin Panter added the comment: I can look at enhancing the tests at some stage, but it isn’t a high priority for me. Regarding translate() with no arguments, it makes sense if you see it as a kind of degenerate case of neither using a translation table, nor any set of bytes to delete:

[issue27506] make bytes/bytearray delete a keyword argument

Serhiy Storchaka added the comment: Technically the patch looks correct to me. Added just few minor comments on Rietveld. I don't think there is a large need in adding the support of keyword argument. But since the overhead is small and somebody needs this, adding this doesn't do a harm. Left

[issue27506] make bytes/bytearray delete a keyword argument

Xiang Zhang added the comment: So let's do a simple benchmark. # without patch ./python -m timeit -s 'string=bytes(range(256));table=bytes(range(255, -1, -1));delete=b"abcdefghijklmn"' 'string.translate(table, delete)' 100 loops, best of 3: 0.55 usec per loop # with patch ./python -m

[issue27506] make bytes/bytearray delete a keyword argument

Martin Panter added the comment: I agree it would be worth checking for a slowdown. As well as giving the option of omitting the table argument, it would make call sites easier to read. It would avoid suggesting that the first argument is translated to the second, like maketrans(). data =

[issue27506] make bytes/bytearray delete a keyword argument

Serhiy Storchaka added the comment: PyArg_ParseTupleAndKeywords can be slower than PyArg_ParseTuple even for positional arguments. We need benchmarking results (especially after committing a patch for issue27574). What is the purpose of adding support of the delete argument as keyword

[issue27506] make bytes/bytearray delete a keyword argument

Martin Panter added the comment: Serhiy, you assigned this to yourself. What do you think of my patch? -- ___ Python tracker ___

[issue27506] make bytes/bytearray delete a keyword argument

Xiang Zhang added the comment: LGTM. Using b'' instead of the None as the default value of *delete* looks better since it doesn't break backwards compatibility. As for the first argument optional or not, actually it's both okay. You have changed the doc accordingly. --

[issue27506] make bytes/bytearray delete a keyword argument

Martin Panter added the comment: This patch is what I had in mind for setting the documented default as delete=b'', but using NULL internally. I also changed it to allow the table argument to be omitted. We can change the documentation accordingly. These are just suggestions; use either or

[issue27506] make bytes/bytearray delete a keyword argument

Xiang Zhang added the comment: Thanks for your comment Martin. I'll apply them later when we reach agreement on functions. I have already used object = NULL, the C default is not necessary here, and it works as you like I think. In patch version 1, b'abc'.translate(None, None) raises

[issue27506] make bytes/bytearray delete a keyword argument

Martin Panter added the comment: Instead of allowing delete=None (which is not in the RST documentation), perhaps it is possible to change the doc string. I can’t remember the details, but I think Argument Clinic allows a virtual Python-level default, something like “object(py_default=b"") =

[issue27506] make bytes/bytearray delete a keyword argument

R. David Murray added the comment: Ah, I was looking at the 2.7 docs. -- nosy: +r.david.murray title: make bytes/bytearray deletechars a keyword argument named delete -> make bytes/bytearray delete a keyword argument ___ Python tracker

[issue27506] make bytes/bytearray delete a keyword argument

Xiang Zhang added the comment: Please review the new version. It makes two changes comparing with the last one. 1. It exposes Python parameter as "delete" (which the document always uses so I think it's the API) while still use "deletechars" (which I prefer as a C variable name) in C code.