-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Functions that are actually classes #5145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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. |
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 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:
|
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. |
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 |
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 What is the benefit of exposing iterators as distinct classes in stubs? This is an implementation detail IMHO. |
The numbers variable needs to be typed with Iteractor[int] to fix a typing error from the change in python/typeshed#5145 . See python/mypy#17414 and python/typeshed#5145 .
@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?
The text was updated successfully, but these errors were encountered: