classification
Title: PySequence_Length() raises TypeError on dict type
Type: behavior Stage: resolved
Components: Documentation, Interpreter Core Versions: Python 3.8, Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Mariatta, ZackerySpytz, docs@python, gvanrossum, mgorny, miss-islington, r.david.murray, serhiy.storchaka, veky
Priority: normal Keywords: patch

Created on 2018-01-06 00:38 by mgorny, last changed 2018-07-23 20:45 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5767 merged ZackerySpytz, 2018-02-20 02:50
PR 5775 closed miss-islington, 2018-02-20 17:25
PR 5776 closed miss-islington, 2018-02-20 17:26
PR 5777 merged miss-islington, 2018-02-20 17:27
PR 5900 merged miss-islington, 2018-02-25 21:13
PR 5901 merged miss-islington, 2018-02-25 21:14
PR 7846 merged serhiy.storchaka, 2018-06-21 14:24
Messages (19)
msg309534 - (view) Author: Michał Górny (mgorny) * Date: 2018-01-06 00:38
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
msg309542 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-06 08:43
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.
msg309543 - (view) Author: Michał Górny (mgorny) * Date: 2018-01-06 08:50
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?
msg309545 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-06 09:49
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.
msg309557 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-06 17:02
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.
msg309577 - (view) Author: Michał Górny (mgorny) * Date: 2018-01-06 20:48
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.
msg309581 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-06 21:16
Since docs and implementation disagree let's call it undefined. I really
can't comment on what PyPy should do.
msg309588 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-06 22:54
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.
msg309607 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-07 05:23
Ah, I like dropping "For objects that do not provide sequence protocol". Go for it!
msg312422 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2018-02-20 17:24
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
msg312430 - (view) Author: miss-islington (miss-islington) Date: 2018-02-20 19:46
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
msg312860 - (view) Author: miss-islington (miss-islington) Date: 2018-02-25 21:19
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
msg312861 - (view) Author: miss-islington (miss-islington) Date: 2018-02-25 21:22
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
msg312864 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2018-02-25 21:29
Thanks!
msg320136 - (view) Author: Vedran Čačić (veky) * Date: 2018-06-21 03:33
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.
msg320177 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2018-06-21 13:19
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.
msg320179 - (view) Author: Vedran Čačić (veky) * Date: 2018-06-21 13:26
Sure, here it is: https://bugs.python.org/issue33933
msg320188 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-06-21 14:30
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.
msg322257 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-23 20:43
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
History
Date User Action Args
2018-07-23 20:45:19serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: resolved
2018-07-23 20:43:44serhiy.storchakasetmessages: + msg322257
2018-06-24 07:09:14serhiy.storchakalinkissue33933 superseder
2018-06-21 14:30:09serhiy.storchakasetstatus: closed -> open
messages: + msg320188

assignee: docs@python -> serhiy.storchaka
components: + Interpreter Core
resolution: fixed -> (no value)
stage: resolved -> (no value)
2018-06-21 14:24:22serhiy.storchakasetpull_requests: + pull_request7456
2018-06-21 13:26:24vekysetmessages: + msg320179
2018-06-21 13:19:41r.david.murraysetnosy: + r.david.murray
messages: + msg320177
2018-06-21 03:33:30vekysetnosy: + veky
messages: + msg320136
2018-02-25 21:29:04Mariattasetstatus: open -> closed
resolution: fixed
messages: + msg312864

stage: patch review -> resolved
2018-02-25 21:22:46miss-islingtonsetmessages: + msg312861
2018-02-25 21:19:57miss-islingtonsetmessages: + msg312860
2018-02-25 21:14:07miss-islingtonsetpull_requests: + pull_request5671
2018-02-25 21:13:00miss-islingtonsetpull_requests: + pull_request5670
2018-02-20 19:46:17miss-islingtonsetnosy: + miss-islington
messages: + msg312430
2018-02-20 17:27:38miss-islingtonsetpull_requests: + pull_request5557
2018-02-20 17:26:40miss-islingtonsetpull_requests: + pull_request5556
2018-02-20 17:25:47miss-islingtonsetpull_requests: + pull_request5554
2018-02-20 17:24:32Mariattasetnosy: + Mariatta
messages: + msg312422
2018-02-20 02:54:21ZackerySpytzsetnosy: + ZackerySpytz

versions: + Python 3.8
2018-02-20 02:50:52ZackerySpytzsetkeywords: + patch
stage: patch review
pull_requests: + pull_request5546
2018-01-07 05:23:34gvanrossumsetmessages: + msg309607
2018-01-06 22:54:16serhiy.storchakasetmessages: + msg309588
2018-01-06 21:16:47gvanrossumsetmessages: + msg309581
2018-01-06 20:48:46mgornysetmessages: + msg309577
2018-01-06 17:02:24gvanrossumsetmessages: + msg309557
2018-01-06 09:49:48serhiy.storchakasetversions: + Python 3.7, - Python 3.4, Python 3.5
nosy: + gvanrossum, docs@python

messages: + msg309545

assignee: docs@python
components: + Documentation, - Extension Modules
2018-01-06 08:50:29mgornysetmessages: + msg309543
2018-01-06 08:43:49serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg309542
2018-01-06 00:38:27mgornycreate