Functions that are actually classes

[hauntsaninja]

@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:

1. Makes typeshed consistent with runtime (albeit undocumented facet of the runtime)
2. Allows users to do things that depend on these being classes, e.g. type checkers will understand isinstance(x, itertools.count) (admittedly rare)

Cons:

1. Change can cause annoying false positives because type checkers are often strict about redefinition. E.g. Fix Generators typeshed test mypy#10255

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?

[hatal175]

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.

[JelleZijlstra]

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, and zip in builtins.

I'm curious how common the con listed by Shantanu actually is; the affected code in the mypy test suite looks pretty contrived.

[hatal175]

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.

[hauntsaninja]

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 function

[srittau]

I 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.

[jakebailey]

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 zip or map as "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).

[hatal175]

So what is the verdict?

[JelleZijlstra]

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.

[Akuli]

#5211 changed most things in itertools, and #5213 changed the rest, #5219 changed builtins.

For people who might be searching for it: multiprocessing has the opposite problem: classes that are actually functions. See #5346 for example.

[eltoder]

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 res as Iterable[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.