-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
Description
It's common nowadays to annotate methods with the abstract Sequence[T]
rather than list[T]
when mutation is not required. However, I noticed that one sometimes encounters / is tempted to write the following pattern:
from collections.abc import Sequence
def test(args: Sequence) -> None:
if not args: # <-- potentially dangerous.
...
Here are some example found on GitHub
- https://github.com/search?q=%20%2F%3A%20Sequence%5C%5B%5Cw%2B%5C%5D%5C)%20-%3E%20%5Cw%2B%3A%5Cs*if%20not%20%5Cw%2B%3A%2F&type=code
- https://github.com/kovidgoyal/kitty/blob/3087cbdcecf8630bc864e46e9ea5d44c24e319bf/kitty/session.py#L450-L451
- https://github.com/superlinked/superlinked/blob/b5127975c685c5436c51137a67b3bc6029b5349f/framework/src/framework/common/util/string_util.py#L38-L39
- https://github.com/Checkmk/checkmk/blob/bfcd60706c06277bc64ef2b76dc637ef2f6b6079/cmk/base/modes/check_mk.py#L1281-L1282
- ...
- https://github.com/search?q=+%2Fif+isinstance%5C%28%5Cw%2B%2C+Sequence%5C%29+and+%5Cw%2B%3A%2F&type=code
- https://github.com/osm-search/Nominatim/blob/196de9e974538ca6cd269a231cd769fbbb917406/src/nominatim_api/logging.py#L94
- https://github.com/pawel-slowik/dpgv4/blob/aafac1b06caf65d984349131c2a13e1865aa3c8f/dpgv4.py#L767
- https://github.com/kynk94/torch-firewood/blob/285a251a3410712fd37ae54d4441b21d823c3d99/firewood/utils/common.py#L39
- ...
This is inherently unsafe at the moment, since it assumes that bool(...)
behaves the same for any subclass of collections.abc.Sequence
as it does with builtins.list
or builtins.tuple
, but nothing stops a custom subtype to implement __bool__
and override this behavior.1
The documentation, both the data model docs and the collections.abc
docs, appears silent about any expected behavior for bool(sequence)
. Given the prevalence of code that uses such patterns, I think one should either:
- Make the boolean behavior on
list
/tuple
officially a part of the specification of sequence types in one way or another.2 - Clarify and document that sequences need not behave like
bool(list)
orbool(tuple)
, so that linter-libraries can justifiably trigger warnings in cases like the code above, and guide the user to useif len(seq) ==
instead.
Footnotes
-
In fact, even currently subclasses of
list
,tuple
andstr
can implement__bool__
if they want to. ↩ -
In particular, the most important bit seems to be that truthiness implies non-emptiness implies that the 0-th element exists. So if one is to specify any behavior, I believe this is the minimal fragment that should be included. Maybe sequence truthiness generally need not work exactly like in
list
andtuple
, but this principle seems fundamental. ↩
Metadata
Metadata
Assignees
Labels
Projects
Status