Repository navigation
Functions that are actually classes #5145
Description
Activity
Do you have other cases in typeshed where this type of decision was made?
I think that typeshed should document types as best it could while type checkers could deal with the fallout.
Personally I'm more of a “Not even in the face of Armageddon. Never compromise.” type of guy, but I get why this would annoy people.I'm not sure we ever explicitly made the decision, but the same situation applies to a few functions written in C that return an iterator, including
map,filter, andzipin builtins.I'm curious how common the con listed by Shantanu actually is; the affected code in the mypy test suite looks pretty contrived.
I think you'll get the same issue if you simply chain calls to those functions while putting results in variables which might be a more common case.
Maybe we try the change and see how badly mypy_primer feels.
For reference, here's the complete list of affected objects:
(mypy) ~/dev/mypy master λ python -m mypy.stubtest --check-typeshed --custom-typeshed ~/dev/typeshed --concise | grep 'is not a function' _operator.attrgetter is not a function _operator.itemgetter is not a function _operator.methodcaller is not a function abc.abstractclassmethod is not a function abc.abstractstaticmethod is not a function asyncio.Future._callbacks is not a function asyncio.futures.Future._callbacks is not a function builtins.copyright is not a function builtins.credits is not a function builtins.exit is not a function builtins.filter is not a function builtins.help is not a function builtins.license is not a function builtins.map is not a function builtins.property.fdel is not a function builtins.property.fget is not a function builtins.property.fset is not a function builtins.quit is not a function builtins.reversed is not a function builtins.zip is not a function collections.deque.__hash__ is not a function contextlib.nullcontext is not a function ctypes.memmove is not a function ctypes.memset is not a function functools.singledispatchmethod.__call__ is not a function itertools.accumulate is not a function itertools.combinations is not a function itertools.combinations_with_replacement is not a function itertools.compress is not a function itertools.count is not a function itertools.dropwhile is not a function itertools.filterfalse is not a function itertools.groupby is not a function itertools.islice is not a function itertools.permutations is not a function itertools.product is not a function itertools.repeat is not a function itertools.starmap is not a function itertools.takewhile is not a function itertools.zip_longest is not a function numbers.Number.__hash__ is not a function operator.attrgetter is not a function operator.itemgetter is not a function operator.methodcaller is not a function pickle.Pickler.persistent_id is not a function pickle.Unpickler.persistent_load is not a functionI usually prefer the stubs to follow the implementation as closely as possible. In my experience that solves more problems than it causes. I think mypy_primer can be a good guide, though.
This isn't totally relevant anymore, but in pylance/pyright, we have a source mapper which tries its best to map stub declarations back to their source (because people really want docstrings, navigation to real code, the works). We had to work around some of the type mismatches by allowing the core types of declarations (class, variable, function, etc) to mismatch.
It's all good now (and arguably okay as the name is the critical thing; "count" is "count"), but I know I was certainly surprised to see some of these typed differently than they are in the real code without an explanation as to why. May be a little surprising for users to see things like
zipormapas "class" in their tooltips when the python docs refer to them as builtin functions (and that reasoning was my guess as to why they were written that way).So what is the verdict?
I agree with @srittau: we should follow the implementation unless there's a compelling enough reason to do otherwise. In this case, the compelling reason could be mypy-primer output indicating that typechecker false positives resulting from the change will be common.
@jakebailey's comment is another useful datapoint; stubs aren't consumed just for type checking, and other tools that use them may need complications to work around mismatches if we don't match the runtime exactly.
Any chance this can be reconsidered and reverted? This breaks common patterns of chaining iterators. I have a fair amount of code of the form
res = itertools.takewhile(take_cond, xs) if filter_cond is not None: res = filter(filter_cond, res) if limit is not None: res = itertools.islice(res, limit)
and so on. Now this fails to type check unless I manually declare
resasIterable[T]for correct T.What is the benefit of exposing iterators as distinct classes in stubs? This is an implementation detail IMHO.
isinstance(x, itertools.count)is code smell. I haven't seen this in the wild.Reacted by Shantanu, Akuli and Huib Piguillet- added a commit that references this issue
on Jun 30, 2024
@hatal175 has been fixing several typeshed definitions and was recently looking at itertools. Many documented-as-functions in itertools are currently functions in typeshed, but actually classes at runtime, e.g.
itertools.count. The proposed change is to make them classes.Pros:
isinstance(x, itertools.count)(admittedly rare)Cons:
My opinion is that we do nothing (except for maybe linking this issue in the allowlists ;-) ), since I think the con has a more real impact than the pros. Curious for others' thoughts?