
On Sun, 23 Jan 2022 at 22:45, Aviram Hassan <aviramyhassan@gmail.com> wrote:
Hi all,
Tl;dr - As mentioned in issue https://bugs.python.org/issue5945,
PySequence_Check
andPyMapping_Check
should be deprecated and discontinued. The reason for this is that they do not provide accurate results and can lead to unexpected behavior. (PyMapping_Check for a list will return True, and PySequence_Check for a mapping that doesn't subclass dict will also return true)
TL;DR counter proposal: just update PySequence_Check and PyMapping_Check to test Py_TPFLAGS_SEQUENCE and whatever the new recommendation is for mappings. No new functions, make the old functions do the right thing. Backwards code compatibility with correct behaviour going forward.
The use cases for the change are as follows:
C-API code that wishes to use the PySequence/PyMapping API and to guarantee correct behavior needs to check it is indeed a valid sequence/mapping
OK, I don't use PyMapping_Check in my C extensions but I do test for sequences. And I use the Python duck typing definition of "valid sequence" in that if PySequence_Check returns True, I can invoke sequence-y methods on it.
And I do not see a problem with dicts passing the test, because in Python they are iterable and can be indexed. It may not look very sensible to use a mapping as a sequence, but duck typing / dynamic typing to me means that if it works in Python, it should work in C.
Static types casting from ambiguous “PyObject” to “PySequence” & “PyMapping” types as provided by PyO3 ( https://pyo3.rs/v0.15.1/conversions/tables.html#argument-types)
OK I have no opinion on this. If you want to introduce new functions for this specific problem, go ahead.
The correct way to determine right now if an object is a sequence, is to check its
Py_TPFLAGS_SEQUENCE
flag and check if it’s a string, bytes or bytearray as they are the only sequences not havingTPFLAGS_SEQUENCE
per specification. This is cumbersome for most non-expert developers not aware of the nuances in the C-API. Having a C-API that provides this check out of the box can ease it for most.
That would be me - I wasn't aware that strings etc didn't pass the sequence test! The extensions I've written were not expecting such, so I never thought to see if they'd work.
However, again I don't see the need for a new function.
A non expert such as myself currently writes
if (PySequence_Check(foo)) { ...
Update PySequence_Check to test the magic flag instead, and I won't notice any difference. A bug in my code (str bytes etc not working) will have been fixed and I didn't have to do anything.
An expert currently writes something like
if (PySequenceCheck(foo) || fooType->tp_flags & Py_TPFLAGS_SEQUENCE)) {
(Code to retrieve fooType skipped, yeah, should use use Py_TYPE() etc)
And again all their current code continues to work without change. The flag test is now unnecessary, but I doubt it is going to be a massive performance hit. And if the writer really does care about performance, they can do versions-specific code.
[ munch ]
Duck typing is further “discouraged”. We do not think of it as an
issue since Python already has idioms and static type checkers that discourage this sort of duck typing.
Wait, what? WHAT?
Please leave this out of your proposal.
--
cheers,
Hugh Fisher