Skip to content

Refactor is_named_instance and refers_to_fullname #11961

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

Merged
merged 2 commits into from
Jan 10, 2022
Merged

Refactor is_named_instance and refers_to_fullname #11961

merged 2 commits into from
Jan 10, 2022

Conversation

sobolevn
Copy link
Member

I saw that refers_to_fullname cannot use our new mypy.types.FINAL_DECORATOR_NAMES constant to work. So, I've decided to allow that.

In the process:

  • I've also simplified is_named_instance function to allow Tuple as well (I use Tuple, because it is used for constants)
  • I've also reused is_named_instance in refers_to_fullname
  • Refactored several tuple-specific names into a new constant

@@ -117,6 +117,15 @@
'typing_extensions.Annotated',
)

# We use this constant in various places when checking `tuple` subtyping:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to your diff, but I wonder why Sized and Collection aren't in here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to open a new issue? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes probably. I guess what this affects is whether a heterogeneous tuple type matches Sized or Collection?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done #11961

@sobolevn
Copy link
Member Author

sobolevn commented Jan 10, 2022

Looks like I've accedentaly noticed a bug.

Here's the test definition of decorated_coroutine:

@coroutine
async def decorated_coroutine() -> int:
return 1

Later it is used as x = yield from decorated_coroutine()

Old error message: # E: "yield from" can't be applied to "AwaitableGenerator[Any, Any, int, Coroutine[Any, Any, int]]"

New error message: # E: "yield from" can't be applied to "Coroutine[Any, Any, int]"

>>> import types
>>> import asyncio
>>> @types.coroutine
... async def decorated_coroutine() -> int:
...     return 1
... 
>>> x = decorated_coroutine()
>>> type(x)
<class 'coroutine'>
>>> asyncio.run(x)
1

>>> async def main():
...   y = decorated_coroutine()
...   async for a in y:
...     print(a)
... 
>>> asyncio.run(main())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/sobolev/Desktop/cpython/Lib/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolev/Desktop/cpython/Lib/asyncio/base_events.py", line 637, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "<stdin>", line 3, in main
TypeError: 'async for' requires an object with __aiter__ method, got coroutine

Double-checking:

>>> import inspect
>>> inspect.isasyncgenfunction(decorated_coroutine)
False
>>> inspect.isasyncgen(decorated_coroutine())
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
False

@JelleZijlstra do you agree?

@JelleZijlstra
Copy link
Member

Sorry, I'm not sure I understand. Note that AwaitableGenerator is a mypy-only type, used for pre-async def asyncio coroutines.

@sobolevn
Copy link
Member Author

Note that AwaitableGenerator is a mypy-only type

Ok, I see. Then, it is not a bug. Thanks!

@JelleZijlstra JelleZijlstra merged commit 9ea6d92 into python:master Jan 10, 2022
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants