In Python 2, PyMapping_Check will return 0 for list objects. In Python
3, it returns 1. Obviously, this makes it rather difficult to
differentiate between mappings and other sized iterables. In addition,
it differs from the behavior of the ``collections.Mapping`` ABC --
isinstance([], collections.Mapping) returns False.

Since most of the PyMapping_* functions don't seem to work properly on
lists, I believe this behavior is erroneous.

The behavior can be seen from a C extension, or if you're lazy, using
ctypes:

Python 2.6.2 (release26-maint, Apr 19 2009, 01:56:41)
[GCC 4.3.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes
>>> ctypes.CDLL('libpython2.6.so').PyMapping_Check(ctypes.py_object([]))
0

Python 3.0.1+ (r301:69556, Apr 15 2009, 15:59:22)
[GCC 4.3.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes
>>> ctypes.CDLL('libpython3.0.so').PyMapping_Check(ctypes.py_object([]))
1

I understand this is indeed unintuitive. The reason list objects support
the C "mapping protocol" in 3.x is that it is how slicing of lists (and
tuples, for that matter) is implemented. Perhaps the documentation
should carry a warning about this.

Unfortunately, right now there is no easy way in C to check that an
object implements a given ABC.

Personally, I think PyMapping_Check and PySequence_Check should be
deprecated and removed. Like their Python counterparts,
operator.isMappingType() and operation.isSequenceType(), they are
unreliable at best in the face of not LBYL and abcs.

Why did the list implementation get changed in Py3.x?  Is it now
necessary for any subscripting type to put the same method in both the
sequence methods and mapping methods?  Was this change necessary?

> Why did the list implementation get changed in Py3.x?  Is it now
> necessary for any subscripting type to put the same method in both the
> sequence methods and mapping methods?  Was this change necessary?

I think it's a case of foolish consistency. In py3k there are no opcodes
dedicated to slicing anymore, instead the slice object is passed to the
mapping's getitem method.

Guido, can we have your take on this?

Here's a code search link:

 http://www.google.com/codesearch?as_q=PyMapping_Check

And here's how it currently used internal to the Python codebase:

$ ack --cc PyMapping_CheckInclude/abstract.h
1125:     PyAPI_FUNC(int) PyMapping_Check(PyObject *o);

Modules/posixmodule.c
3585:    if (!PyMapping_Check(env)) {
3764:    if (!PyMapping_Check(env)) {
3950:    if (!PyMapping_Check(env)) {

Objects/abstract.c
1987:PyMapping_Check(PyObject *o)

Objects/frameobject.c
605:        (locals != NULL && !PyMapping_Check(locals))) {

PC/_subprocess.c
340:    if (! PyMapping_Check(environment)) {

Python/bltinmodule.c
700:    if (locals != Py_None && !PyMapping_Check(locals)) {
705:        PyErr_SetString(PyExc_TypeError, PyMapping_Check(globals) ?
794:    if (!PyMapping_Check(locals)) {

> Why did the list implementation get changed in Py3.x?

Because we decided to get rid of the sq_slice and sq_ass_slice slots
in PySequenceMethods, and that in turn was because we got rid of the
slice-related opcodes and the separate __getslice__ and __setslice__
magic methods.

> Is it now necessary for any subscripting type to put the same method
> in both the sequence methods and mapping methods?

Yes, if the type wants to support slicing.  The reason is that while
we changed many things, we didn't want to change the signature of the
methods that we kept, and the sq_item/sq_ass_item signatures have
arguments that make it impossible to pass the info necessary for a
slice.

> Was this change necessary?

The changes are briefly mentioned in PEP 3100 without any motivation.
However I think the rationale was the observation that the old
sq_slice / sq_ass_slice took only integers (really ssize_t), meaning
that it was impossible to implement a sequence type taking a slice
with arguments outside the range supported by ssize_t (e.g. a custom
range type supporting huge longs).  Or with non-integral arguments.
This problem never existed for non-slice __get__ since one could
always implement mp_subscript / mp_ass_subscript.

Was it necessary?  I'm not sure	-- but that's water under the bridge. 	

Was it a good change?  From the	POV of Python code, yes.  The old
approach caused some	odd problems for classes implementing
__getslice__ / __setslice__ (since those were invoked after the
arguments had been pushed through the sq_slice / sq_ass_slice API).

> Personally, I think PyMapping_Check and PySequence_Check should be
> deprecated and removed. Like their Python counterparts,
> operator.isMappingType() and operation.isSequenceType(), they are
> unreliable at best in the face of not LBYL and abcs.

Right, calling PyMapping_Check() was never particularly reliable, and
extension modules depending on it probably always had subtle bugs. 

Perhaps it would be nice if we provided a C API to at least some of
the ABC package.

> Right, calling PyMapping_Check() was never particularly reliable, and
> extension modules depending on it probably always had subtle bugs. 

> Perhaps it would be nice if we provided a C API to at least some of
> the ABC package.

In the meantime, would it be reasonable to add the moral equivalent of `hasattr(type(op), 'items')` to PyMapping_Check()?

> In the meantime, would it be reasonable to add the moral equivalent
> of `hasattr(type(op), 'items')` to PyMapping_Check()?

That all depends on what it is used for.  Which is hard to say without someone following more of the links that Raymond posted.

Modules/posixmodule.c: uses PyMapping_Size(), PyMapping_Keys() and PyMapping_Values() to parse an environment mapping (for execve() and friends).

PyFrame_New(): validates the "locals" argument in pydebug mode (only used for module-level code).
Note that functions in frameobject.c have "assert(PyDict_Check(dict))" where "dict" is that same locals argument (copied into f_locals)...

PC/_subprocess.c: uses PyMapping_Size(), PyMapping_Keys() and PyMapping_Values() to parse an environment mapping (for CreateProcessW()).

Python/btninmodule.c: validates the "locals" argument to eval().


There are also a couple of places where the PyMapping API (such PyMapping_Keys()) is used (e.g. _json), but without calling PyMapping_Check first.

The question is, if PyMapping_Check() returns True, and a list is passed, will the code segfault or raise an exception?  A segfault would be unacceptable; an exception would be acceptable assuming that the code would have raised an exception anyway if PyMapping_Check() had returned False.

Exceptions seem to be raised (for code that can be exercised in Python), but not very obvious ones:

>>> eval("x", {}, [])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <module>
TypeError: list indices must be integers, not str

$ ./python -c "import os; os.execle('/bin/ls', 'ls', [])"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/antoine/py3k/__svn__/Lib/os.py", line 320, in execle
    execve(file, args[:-1], env)
AttributeError: values

It looks like PyMapping_Check() already checks for the presence of a fairly arbitrary special operation (mp_subscript).  It sounds fine to replace that with a check for the presence of a keys() or items() method (I'm not sure which one is more representative). I wish the check can be done fast -- but I fear that it'll involve a dict lookup.  So be it.

Actually, making PyMapping_Check() more robust produces a test failure in test_builtin, because of the following code:

        # Verify locals stores (used by list comps)
        eval('[locals() for i in (2,3)]', g, d)
        eval('[locals() for i in (2,3)]', g, collections.UserDict())

        class SpreadSheet:
            "Sample application showing nested, calculated lookups."
            _cells = {}
            def __setitem__(self, key, formula):
                self._cells[key] = formula
            def __getitem__(self, key):
                return eval(self._cells[key], globals(), self)

        ss = SpreadSheet()
        ss['a1'] = '5'
        ss['a2'] = 'a1*6'
        ss['a3'] = 'a2*7'
        self.assertEqual(ss['a3'], 210)


Should I fix the example to include a keys() method?

Rather than introduce "fixes" that break code and hurt performance, I think it would be better to deprecate PyMapping_Check() and wait for a fast, clean C version of the ABCs (that is supposed to be our one obvious way to do it).

FWIW, the spreadsheet example has been around for years and I know of more than one private company that has made heavy use of code modeled on that example (not for spreadsheets, but as a hook for eval).  So, I don't think the new "keys" check should be backported.

> Rather than introduce "fixes" that break code and hurt performance, I
> think it would be better to deprecate PyMapping_Check() and wait for a
> fast, clean C version of the ABCs (that is supposed to be our one
> obvious way to do it).

Do you also advocate deprecating PySequence_Check()? Both are useful
APIs to know what you're dealing with.

As for the "clean C version of the ABCs", I'm afraid we could wait quite
a bit, since that's a lot more work and nobody seems really interested
in the matter.

> FWIW, the spreadsheet example has been around for years and I know of
> more than one private company that has made heavy use of code modeled
> on that example (not for spreadsheets, but as a hook for eval).  So, I
> don't think the new "keys" check should be backported.

Well, I'm not proposing to backport it, but to fix things in 3.2.

> Do you also advocate deprecating PySequence_Check()? 

Perhaps just document PyMapping_Check() as being less informative than before (now it has false positives for sequences).

> As for the "clean C version of the ABCs", 
> I'm afraid we could wait quite a bit

That may be true.  I hope not.  The ABCs were meant to solve exactly this problem.  At this point, I would rather ignore the problem for 3.2 than to implement a less clean alternative.

> > As for the "clean C version of the ABCs", 
> > I'm afraid we could wait quite a bit
> 
> That may be true.  I hope not.  The ABCs were meant to solve exactly
> this problem.  At this point, I would rather ignore the problem for
> 3.2 than to implement a less clean alternative.

Also, please note that checking for ABCs would not solve the
test_builtin failure, since the class there doesn't implement the
Mapping ABC (and doesn't claim to either).

We've hit this problem today.

What are we supposed to do in the meantime?

Not use PyMapping_Check?

On Mon, Feb 2, 2015, at 14:02, Buck Golemon wrote:
> 
> Buck Golemon added the comment:
> 
> We've hit this problem today.
> 
> What are we supposed to do in the meantime?
> 
> ----------
> nosy: +bukzor
> 
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue5945>
> _______________________________________

Proposed wording doesn't looks much informative to me. Maybe just say that PyMapping_Check() also returns 1 for sequences that support slicing? And recommend `PyMapping_Check() && !PySequence_Check()` for true mapping test?

I propose an alternate PR 7029 which also adds other improvements to the documentation of mappings and sequences C API.

New changeset f5b1183610d5888db3bbd639b1a0c945dbd8f8dd by Serhiy Storchaka in branch 'master':
bpo-5945: Improve mappings and sequences C API docs. (GH-7029)
https://github.com/python/cpython/commit/f5b1183610d5888db3bbd639b1a0c945dbd8f8dd

New changeset e1a78cacf65f007b1000966ce3aac6ac2eaa5cfc by Miss Islington (bot) in branch '3.7':
bpo-5945: Improve mappings and sequences C API docs. (GH-7029)
https://github.com/python/cpython/commit/e1a78cacf65f007b1000966ce3aac6ac2eaa5cfc

New changeset 93e9fb5664e56c02c9aa89098b556929735b35db by Serhiy Storchaka in branch '3.6':
[3.6] bpo-5945: Improve mappings and sequences C API docs. (GH-7029). (GH-7049)
https://github.com/python/cpython/commit/93e9fb5664e56c02c9aa89098b556929735b35db

Are these changes enough? Can this issue be closed now?