FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Allow function arguments as base classes by ilevkivskyi · Pull Request #14135 · python/mypy · GitHub

Repository navigation

Allow function arguments as base classes - #14135

Merged
ilevkivskyi merged 4 commits into
python:masterfrom
ilevkivskyi:allow-arg-bases
Nov 21, 2022
Merged

ilevkivskyi merged 4 commits into
python:masterfrom
ilevkivskyi:allow-arg-bases

Conversation

ilevkivskyi commented Nov 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #5865

Looks quite easy and safe, unless I am missing something. Most changes in the diff are just moving stuff around.

Previously we only applied argument types before type checking, but it looks like we can totally do this in semantic analyzer. I also enable variable annotated as type (or equivalently Type[Any]), this use case was mentioned in the comments.

This PR also accidentally fixes two additional bugs, one related to type variables with values vs walrus operator, another one for type variables with values vs self types. I include test cases for those as well.

This comment has been minimized.

Copy link
Copy Markdown
Member Author

The homeassistant errors are caused by exposing an existing bug in walrus. On master this test

[case testNarrowOnSelfInGeneric]
# flags: --strict-optional
from typing import Generic, TypeVar, Optional

T = TypeVar("T", int, str)

class C(Generic[T]):
    x: Optional[T]
    def meth(self) -> Optional[T]:
        if (y := self.x) is not None:
            return reveal_type(y)
        return None

reveals type Any. But with this PR it becomes optional, so it is still wrong. I will first try to figure out what is going on.

Copy link
Copy Markdown
Member Author

OK, I have a fix for the walrus bug, since it is an obvious bug, I am going to just add it to this PR.

This comment has been minimized.

Copy link
Copy Markdown
Member Author

Looking again at mypy_primer it looks like this PR accidental fixes another bug related to type vars with values, this type for self types. I am going to include a regression for that one as well.

Copy link
Copy Markdown
Contributor

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

artigraph (https://github.com/artigraph/artigraph)
+ tests/arti/producers/test_producer.py:679: error: Unused "type: ignore" comment
+ tests/arti/producers/test_producer.py:680: error: Unused "type: ignore" comment

Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/dependencies/reloaders.py:566: error: Cannot determine type of "real_path"  [has-type]
- tanjun/dependencies/reloaders.py:567: error: Cannot determine type of "time"  [has-type]
- tanjun/dependencies/reloaders.py:578: error: Cannot determine type of "info"  [has-type]
- tanjun/dependencies/reloaders.py:579: error: Cannot determine type of "time"  [has-type]
- tanjun/dependencies/reloaders.py:599: error: Cannot determine type of "path"  [has-type]
- tanjun/dependencies/reloaders.py:600: error: Cannot determine type of "tracked_value"  [has-type]
- tanjun/dependencies/reloaders.py:609: error: Cannot determine type of "path"  [has-type]
- tanjun/dependencies/reloaders.py:609: error: Cannot determine type of "path_info"  [has-type]
- tanjun/commands/menu.py:580: error: Incompatible return value type (got "MenuCommand[_MenuCommandCallbackSigT, _MenuTypeT]", expected "Self")  [return-value]

mypy (https://github.com/python/mypy)
+ mypy/stubtest.py:357: error: Unused "type: ignore[valid-type]" comment

cloud-init (https://github.com/canonical/cloud-init)
+ cloudinit/templater.py:44: error: Unused "type: ignore" comment

JukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nice -- fixing three bugs for the price of one!

I can't see why setting the argument types in the semantic analyzer would be a problem.

Left a comment that is mostly unrelated to this PR, feel free to ignore it if you are not sure.

Comment thread mypy/types.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

Function argument rejected as a base class

2 participants


Back | FazBrowse Home | New Git URL