While debugging PyPy test failure on backports.lzma [1], I've noticed that PySequence_Check() on a dict type raises TypeError, e.g.:

  Traceback (most recent call last):
    File "test/test_lzma.py", line 273, in test_bad_args
      b"", format=lzma.FORMAT_RAW, filters={})
    File "/home/mgorny/git/backports.lzma/build/lib.linux-x86_64-3.6/backports/lzma/__init__.py", line 463, in decompress
      decomp = LZMADecompressor(format, memlimit, filters)
  TypeError: object of type 'dict' has no len()

The relevant C code is:

  static int
  parse_filter_chain_spec(lzma_filter filters[], PyObject *filterspecs)
  {
    Py_ssize_t i, num_filters;

    num_filters = PySequence_Length(filterspecs);
    ...

where filterspecs is the object corresponding to the {} dict in Python snippet.

According to the documentation [2], PySequence_Length() should be 'equivalent to the Python expression len(o).' The Python expression obviously does not raise TypeError:

  >>> len({})
  0

Therefore, I think that the behavior of PySequence_Length() is a bug, and the function should successfully return the dict length instead.

[1]:https://github.com/peterjc/backports.lzma
[2]:https://docs.python.org/3/c-api/sequence.html#c.PySequence_Length

PySequence_Length(o) is equivalent to the Python expression len(o) only if o is a sequence. dict is not a sequence. You must use a correct API:

PyObject_Size() -- for general objects.
PyMapping_Size() -- if the object is a mapping.
PyDict_Size() -- if the object is a concrete dict.

So is the documentation mistaken or just confusing? It says straight:

> For objects that do not provide sequence protocol, this is equivalent to the Python expression len(o).

So it the 'do not' mistaken or does it mean objects that are sequences but do not provide the sequence protocol?

Agreed, it looks either mistaken or confusing. This wording is used from the first versions of the C API documentation. I don't know what is meant here.

If we were to take the docs literally then PySequence_{Size,Length} should just be aliases for PyObject_Size.  But I'm reluctant to change something that has been like this for ages.

In fact maybe we should deprecate them, together with other PySequence_ and PyMapping_ operations that have more general PyObject_ alternatives?

You can read the source code to find the root cause for the behavior -- PySequence_{Size,Length} only looks for tp_as_sequence->sq_length, but dict has that field set to NULL.  (A dict's size is computed by tp_as_mapping->mp_length.)

Fixing dict by adding a redundant sq_length seems brittle (there may be many other mapping types with similar issues).

So I think we should update the docs to recommend PyObject_Size instead.

Well, my two main concerns are:

1) whether it is acceptable for PyPy to not raise TypeError in this case, or if I should report it to PyPy upstream as a bug,

2) and whether programmers can appropriately rely on PySequence_Length() raising TypeError for non-sequence types or if they should use e.g. PySequence_Check() explicitly.

Since docs and implementation disagree let's call it undefined. I really
can't comment on what PyPy should do.

Note that many other PySequence_* functions support only sequences. For example PySequence_Count(o1, o2) is the equivalent of the Python expression o1 + o2 only if o1 is a sequence. If pass integer objects to PySequence_Count() it will return an error.

I would remove "do not" from the documentation. Or even the whole "For objects that do not provide sequence protocol" since this is implied.

Ah, I like dropping "For objects that do not provide sequence protocol". Go for it!

New changeset 7a1e1786f98ad49caa157dcdf14ada9d0b07d0fd by Mariatta (Zackery Spytz) in branch 'master':
bpo-32500: Correct the documentation for PySequence_Size() and PySequence_Length() (GH-5767)
https://github.com/python/cpython/commit/7a1e1786f98ad49caa157dcdf14ada9d0b07d0fd

New changeset 6ae87cae091f4835090c10c1e65eb057a13fca2c by Miss Islington (bot) in branch '3.6':
bpo-32500: Correct the documentation for PySequence_Size() and PySequence_Length() (GH-5767)
https://github.com/python/cpython/commit/6ae87cae091f4835090c10c1e65eb057a13fca2c

New changeset 139e64694f546982a8cc801dda30670abadb8d06 by Miss Islington (bot) in branch '3.7':
bpo-32500: Correct the documentation for PySequence_Size() and PySequence_Length() (GH-5767)
https://github.com/python/cpython/commit/139e64694f546982a8cc801dda30670abadb8d06

New changeset ecaa372f74eeb6c032791c520af6b23e527d335f by Miss Islington (bot) in branch '2.7':
bpo-32500: Correct the documentation for PySequence_Size() and PySequence_Length() (GH-5767)
https://github.com/python/cpython/commit/ecaa372f74eeb6c032791c520af6b23e527d335f

Thanks!

I don't believe this. Are you seriously saying that it is ok to tell the user "object of type 'dict' has no len()" when of course it does? Please change the error message so it does not so blatantly lie. Yes, I understand the underlying issue, but the common user doesn't need to understand it, and they shouldn't be lied to.

IIUC that error message came from pypy, not CPython.  If you have a reproducer for CPython, you can open a new issue with a request to fix the error message.

Sure, here it is: https://bugs.python.org/issue33933

I agree that error messages sometimes is misleading if use the C API.

PR 7846 fixes this, although I'm not sure this is the best wording.

New changeset a6fdddb7df00aefad2ec6e362dbf10d4bd8bff32 by Serhiy Storchaka in branch 'master':
bpo-32500: Fix error messages for sequence and mapping C API. (GH-7846)
https://github.com/python/cpython/commit/a6fdddb7df00aefad2ec6e362dbf10d4bd8bff32