Skip to content
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.

Sign up for GitHub

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

Jump to bottom

improve inspect.pyi #5473

Merged
merged 10 commits into from
May 29, 2021
Merged

improve inspect.pyi #5473

merged 10 commits into from
May 29, 2021

Conversation

JelleZijlstra
Copy link
Member

@JelleZijlstra
improve inspect.pyi
eedf5bd 
- Use TypeGuard for various is* functions (refer to python#5406)
- Use collections.abc and builtin containers
@JelleZijlstra
type args for async gen type
3850df2
srittau
srittau approved these changes May 16, 2021
View reviewed changes
Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

LGTM, except for pyright's warning and the marked spot.

stdlib/inspect.pyi Outdated

# Create private type alias to avoid conflict with symbol of same
# Create private type alias to avoid confdict with symbol of same
Copy link
Collaborator

Choose a reason for hiding this comment

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

^^

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, thanks

@JelleZijlstra
Copy link
Member Author

Looks like mypy crashes on TypeGuard[Type[Any]]. I'll investigate a bit more and probably report a mypy issue and for now keep isclass returning bool.

JelleZijlstra added a commit to python/mypy that referenced this pull request May 16, 2021
@JelleZijlstra
Remove assertion from TypeGuard serialization
a94dc07 
This came up in python/typeshed#5473: mypy started crashing when
I made a function return TypeGuard[Type[Any]]. I wasn't able to
reproduce this locally so far, but verified that removing the
assertion fixes the crash.
@JelleZijlstra JelleZijlstra mentioned this pull request May 16, 2021
Merged
@JelleZijlstra JelleZijlstra mentioned this pull request May 16, 2021
Merged
@srittau srittau mentioned this pull request May 16, 2021
Merged
@JelleZijlstra
Copy link
Member Author

Primer still fails to leave a comment.

@srittau
Copy link
Collaborator

srittau commented May 16, 2021

I reset the permissions at https://github.com/python/typeshed/settings/actions and restarted the CI run.

@srittau
Copy link
Collaborator

srittau commented May 16, 2021

And since that had no effect, I set the stricter permissions again. I'm trying something else.

@srittau
Copy link
Collaborator

srittau commented May 17, 2021

🤞

@github-actions

This comment has been minimized.

Sign in to view
@JelleZijlstra
Copy link
Member Author

Well, it worked! But now i need to figure out what all those changes are about.

For reference: https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/util/inspect.py#L252, https://github.com/streamlit/streamlit/blob/develop/lib/streamlit/elements/doc_string.py#L105

Looks like mypy has some bug around a TypeGuard anded with another if check.

This was referenced May 17, 2021
Closed
Merged
@JelleZijlstra JelleZijlstra added the status: deferred Issue or PR deferred until some precondition is fixed label May 18, 2021
JukkaL pushed a commit to python/mypy that referenced this pull request May 19, 2021
@JelleZijlstra
Remove assertion from TypeGuard serialization (#10486)
02e016f 
This came up in python/typeshed#5473: mypy started crashing when
I made a function return TypeGuard[Type[Any]]. I wasn't able to
reproduce this locally so far, but verified that removing the
assertion fixes the crash.
JelleZijlstra added a commit to python/mypy that referenced this pull request May 21, 2021
@JelleZijlstra
Fix crash on TypeGuard plus "and" (#10496)
8e909e4 
In python/typeshed#5473, I tried to switch a number of `inspect` functions to use the new `TypeGuard` functionality. Unfortunately, mypy-primer found a number of crashes in third-party libraries in places where a TypeGuard function was ANDed together with some other check. Examples:

- https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/util/inspect.py#L252
- https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/ext/coverage.py#L212
- https://github.com/streamlit/streamlit/blob/develop/lib/streamlit/elements/doc_string.py#L105

The problems trace back to the decision in #9865 to make TypeGuardType not inherit from ProperType: in various conditions that are more complicated than a simple `if` check, mypy wants everything to become a ProperType. Therefore, to fix the crashes I had to make TypeGuardType a ProperType and support it in various visitors.
@JelleZijlstra
Copy link
Member Author

Made this include #5551 to see what happens with mypy-primer.

@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

sphinx (https://github.com/sphinx-doc/sphinx.git)
+ sphinx/util/inspect.py: note: In function "is_singledispatch_function":
+ sphinx/util/inspect.py:334:13: error: "FunctionType" has no attribute "dispatch"
+ sphinx/util/inspect.py: note: At top level:
+ sphinx/ext/coverage.py: note: In member "build_py_coverage" of class "CoverageBuilder":
+ sphinx/ext/coverage.py:218:51: error: Item "object" of "Union[object, object]" has no attribute "__doc__"

graphql-core (https://github.com/graphql-python/graphql-core.git)
+ src/graphql/subscription/subscribe.py:89: error: unused "type: ignore" comment
+ tests/execution/test_sync.py:61: error: Redundant cast to "Awaitable[Any]"

@JelleZijlstra
Copy link
Member Author

The coverage.py one is still a bug in mypy (https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/ext/coverage.py#L218). It somehow narrows the type down from Any to object.

srittau
srittau approved these changes May 29, 2021
View reviewed changes
Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

The impact of the mypy bug seems to be rather small. The problem with inspect.py is unfortunate, but a standard problem with mypy not understanding hasattr().

@srittau srittau merged commit b2e3c4f into python:master May 29, 2021
@JelleZijlstra JelleZijlstra deleted the inspect branch May 29, 2021 17:32
@JelleZijlstra JelleZijlstra mentioned this pull request May 29, 2021
Open
A5rocks added a commit to A5rocks/hikari that referenced this pull request Jun 20, 2021
@A5rocks
Use TypeGuards instead
6250777 
Other two type ignores here are pending update of typeshed in mypy:
python/typeshed#5473
A5rocks added a commit to A5rocks/hikari that referenced this pull request Jul 30, 2021
@A5rocks
Use TypeGuards instead
d199137 
Other two type ignores here are pending update of typeshed in mypy:
python/typeshed#5473
davfsa added a commit to hikari-py/hikari that referenced this pull request Aug 6, 2021
@A5rocks @davfsa @FasterSpeeding
Remove a couple type ignores + cleanup CacheMappingView (#657)
88dce72 
* Work around a MyPy inference (? or typeshed) bug

* Clean up CacheMappingView

* A few more type ignores

* Please flake8

* Revert isinstance checks

* Use TypeGuards instead

Other two type ignores here are pending update of typeshed in mypy:
python/typeshed#5473

* Code review feedback

Co-authored-by: davfsa <davfsa@gmail.com>
Co-authored-by: FasterSpeeding <lucina@lmbyrne.dev>

* Use mypy attrs plugin more effectively

* Fix merge conflicts

* Remove redundancy in config

* Re-order type overloads and abstract method decorators

Now, this PR follows the convention found elsewhere in the library.
(In hikari/api/rest.py)

* Actually run converters, as well as have a docstring for ssl

* davfsa patch to cache, with a couple changes

* Don't run converters on setattr

* Fix Cache3DMappingView

(why isn't mypy warning me about this!!)

* Remove transparent mapping

* Remove redundant else clause

* Reformat

Co-authored-by: davfsa <davfsa@gmail.com>
Co-authored-by: FasterSpeeding <lucina@lmbyrne.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srittau srittau srittau approved these changes

Assignees
No one assigned
Labels
status: deferred Issue or PR deferred until some precondition is fixed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JelleZijlstra @srittau