Skip to content

infer types from explicitly-typed base if not explicitly typed #12277

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

Closed
wants to merge 5 commits into from

Conversation

Riolku
Copy link

@Riolku Riolku commented Mar 2, 2022

Fixes #12268. If the type of an lvalue is not given at definition time, attempt to infer it from the type of a base member.

Fixes python#12268. If the type of an lvalue is not given at definition time, 
attempt to infer it from the type of a base member.
@Riolku Riolku marked this pull request as draft March 2, 2022 18:25
@github-actions

This comment has been minimized.

Also change test output for one of the tests.
@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

Obviously it's clear from the diff that I've modified the output for one of the tests. I looked into when it was created: #5670. This references partial types. However, (and please forgive me if I'm wrong, as it's my first time contributing) I believe Partial types are those that have something to do with None, not []. Since this PR uses parent types as a fallback if forced to infer, that test necessarily fails. But what does it test?

@Riolku Riolku marked this pull request as ready for review March 3, 2022 01:00
@github-actions

This comment has been minimized.

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

After looking at the sheer volume of the mypy_primer diff, I think I should take a different approach. For instance, consider the following:

class A:
  x = SuperType()

class B(A):
  x = DerivedType()

I think it's more reasonable to expect that B.x is of type DerivedType, rather than SuperType. I think an ideal PR would look more like "If the base class is explicitly typed, use it", rather than the current, which always takes it if it has to infer.

@Riolku Riolku marked this pull request as draft March 3, 2022 02:18
@Riolku Riolku marked this pull request as ready for review March 3, 2022 03:02
@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

Hopefully this will do it. This also means I reverted the change to the test I mentioned.

@Riolku Riolku changed the title infer types from base if not explicitly typed infer types from explicitly-typed base if not explicitly typed Mar 3, 2022
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

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

steam.py (https://github.com/Gobot1234/steam.py)
+ steam/protobufs/headers.py:158: error: Value of type "Union[str, Iterable[str]]" is not indexable  [index]
+ steam/protobufs/headers.py:158: error: Incompatible types in assignment (expression has type "Union[str, Any]", base class "BaseMsgHdr" defined the type as "Tuple[str, ...]")  [assignment]

aioredis (https://github.com/aio-libs/aioredis)
+ aioredis/connection.py:367: error: Unsupported left operand type for + ("Iterable[str]")  [operator]
+ aioredis/connection.py:447: error: Unsupported left operand type for + ("Iterable[str]")  [operator]

urllib3 (https://github.com/urllib3/urllib3)
+ src/urllib3/connectionpool.py:1033: error: Unexpected keyword argument "cert_file" for "HTTPConnection"  [call-arg]
+ src/urllib3/connection.py:134: note: "HTTPConnection" defined here
+ src/urllib3/connectionpool.py:1033: error: Unexpected keyword argument "key_file" for "HTTPConnection"  [call-arg]
+ src/urllib3/connection.py:134: note: "HTTPConnection" defined here
+ src/urllib3/connectionpool.py:1033: error: Unexpected keyword argument "key_password" for "HTTPConnection"  [call-arg]
+ src/urllib3/connection.py:134: note: "HTTPConnection" defined here
+ src/urllib3/connectionpool.py:1043: error: Argument 1 to "_prepare_conn" of "HTTPSConnectionPool" has incompatible type "HTTPConnection"; expected "HTTPSConnection"  [arg-type]

edgedb (https://github.com/edgedb/edgedb)
+ edb/schema/indexes.py:397:16: error: Unexpected keyword argument "expr" for "ObjectDDL"

sympy (https://github.com/sympy/sympy)
+ sympy/codegen/ast.py:1449: error: No overload variant of "__add__" of "tuple" matches argument type "Iterable[str]"
+ sympy/codegen/ast.py:1449: note: Possible overload variants:
+ sympy/codegen/ast.py:1449: note:     def __add__(self, Tuple[str, ...]) -> Tuple[str, ...]
+ sympy/codegen/ast.py:1449: note:     def [_T] __add__(self, Tuple[_T, ...]) -> Tuple[Union[str, _T], ...]
+ sympy/codegen/ast.py:1816: error: Value of type "Iterable[str]" is not indexable
+ sympy/polys/domains/integerring.py:38: error: "None" not callable
+ sympy/polys/domains/integerring.py:39: error: "None" not callable
+ sympy/polys/domains/rationalfield.py:41: error: "None" not callable
+ sympy/polys/domains/rationalfield.py:42: error: "None" not callable
+ sympy/polys/domains/pythonintegerring.py:21: error: "None" not callable
+ sympy/polys/domains/pythonintegerring.py:22: error: "None" not callable
+ sympy/polys/domains/gmpyintegerring.py:22: error: "None" not callable
+ sympy/polys/domains/gmpyintegerring.py:23: error: "None" not callable
+ sympy/polys/domains/pythonrationalfield.py:18: error: "None" not callable
+ sympy/polys/domains/pythonrationalfield.py:19: error: "None" not callable
+ sympy/polys/domains/gmpyrationalfield.py:21: error: "None" not callable
+ sympy/polys/domains/gmpyrationalfield.py:22: error: "None" not callable
+ sympy/polys/domains/gaussiandomains.py:428: error: "None" not callable
+ sympy/polys/domains/gaussiandomains.py:429: error: "None" not callable
+ sympy/polys/domains/gaussiandomains.py:430: error: "None" not callable
+ sympy/polys/domains/gaussiandomains.py:431: error: Unsupported operand type for unary - ("Optional[Any]")
+ sympy/polys/domains/gaussiandomains.py:599: error: "None" not callable
+ sympy/polys/domains/gaussiandomains.py:600: error: "None" not callable
+ sympy/polys/domains/gaussiandomains.py:601: error: "None" not callable
+ sympy/polys/domains/gaussiandomains.py:602: error: Unsupported operand type for unary - ("Optional[Any]")

ibis (https://github.com/ibis-project/ibis)
+ ibis/expr/datatypes/__init__.py:82: error: Unsupported left operand type for + ("Iterable[str]")
+ ibis/expr/datatypes/__init__.py:91: error: Unsupported left operand type for + ("Iterable[str]")
- ibis/backends/base/sql/alchemy/__init__.py:61: error: Incompatible types in assignment (expression has type "Type[AlchemyTable]", base class "BaseSQLBackend" defined the type as "Type[DatabaseTable]")
- ibis/backends/base/sql/alchemy/__init__.py:357: error: Cannot determine type of "table_class"
- ibis/tests/expr/mocks.py:409: error: Incompatible types in assignment (expression has type "Type[AlchemyTable]", base class "BaseSQLBackend" defined the type as "Type[DatabaseTable]")
- ibis/backends/sqlite/__init__.py:108: note:          def table(self, name: str, database: Optional[str] = ..., schema: Optional[str] = ...) -> TableExpr
- ibis/backends/sqlite/__init__.py:108: note:          def table(self, name: str, database: Optional[str] = ...) -> TableExpr
- ibis/backends/sqlite/__init__.py:124: error: Cannot determine type of "table_class"

bandersnatch (https://github.com/pypa/bandersnatch)
+ src/bandersnatch_storage_plugins/swift.py: note: In member "walk" of class "SwiftStorage":
+ src/bandersnatch_storage_plugins/swift.py:624: error: Argument 1 to "append" of "list" has incompatible type "Path"; expected "SwiftPath"
+ src/bandersnatch_storage_plugins/swift.py:626: error: Argument 1 to "append" of "list" has incompatible type "Path"; expected "SwiftPath"
+ src/bandersnatch_storage_plugins/swift.py: note: In member "rmdir" of class "SwiftStorage":
+ src/bandersnatch_storage_plugins/swift.py:839: error: Unexpected keyword argument "include_swiftkeep" for "iterdir" of "Path"
+ src/bandersnatch_storage_plugins/swift.py:839: error: Unexpected keyword argument "recurse" for "iterdir" of "Path"
+ src/bandersnatch_storage_plugins/swift.py:839: error: Unexpected keyword argument "include_swiftkeep" for "iterdir" of "Path"
+ src/bandersnatch_storage_plugins/swift.py:839: error: Unexpected keyword argument "recurse" for "iterdir" of "Path"
+ src/bandersnatch_storage_plugins/swift.py: note: In member "path_backend" of class "SwiftFileLock":
+ src/bandersnatch_storage_plugins/swift.py:64: error: Incompatible return value type (got "Type[Path]", expected "Type[SwiftPath]")
+ src/bandersnatch_storage_plugins/s3.py: note: In member "path_backend" of class "S3FileLock":
+ src/bandersnatch_storage_plugins/s3.py:79: error: Incompatible return value type (got "Type[Path]", expected "Type[S3Path]")
+ src/bandersnatch_storage_plugins/s3.py: note: In member "walk" of class "S3Storage":
+ src/bandersnatch_storage_plugins/s3.py:181: error: Argument 1 to "append" of "list" has incompatible type "Path"; expected "S3Path"
+ src/bandersnatch_storage_plugins/s3.py:183: error: Argument 1 to "append" of "list" has incompatible type "Path"; expected "S3Path"
+ src/bandersnatch_storage_plugins/s3.py:185: error: Argument 1 to "append" of "list" has incompatible type "Path"; expected "S3Path"
+ src/bandersnatch_storage_plugins/s3.py: note: In member "copy_file" of class "S3Storage":
+ src/bandersnatch_storage_plugins/s3.py:249: error: "Path" has no attribute "_accessor"
+ src/bandersnatch_storage_plugins/s3.py:252: error: "Path" has no attribute "key"
+ src/bandersnatch_storage_plugins/s3.py:253: error: "Path" has no attribute "bucket"
+ src/bandersnatch_storage_plugins/s3.py:253: error: "Path" has no attribute "key"
+ src/bandersnatch_storage_plugins/s3.py:254: error: "Path" has no attribute "bucket"
+ src/bandersnatch_storage_plugins/s3.py: note: In member "mkdir" of class "S3Storage":
+ src/bandersnatch_storage_plugins/s3.py:330: error: "Type[Path]" has no attribute "keep_file"
+ src/bandersnatch_storage_plugins/s3.py:335: error: "Type[Path]" has no attribute "keep_file"
+ src/bandersnatch_storage_plugins/s3.py: note: In member "get_upload_time" of class "S3Storage":
+ src/bandersnatch_storage_plugins/s3.py:397: error: "Path" has no attribute "_accessor"
+ src/bandersnatch_storage_plugins/s3.py:398: error: "Path" has no attribute "bucket"
+ src/bandersnatch_storage_plugins/s3.py:398: error: "Path" has no attribute "key"
+ src/bandersnatch_storage_plugins/s3.py: note: In member "set_upload_time" of class "S3Storage":
+ src/bandersnatch_storage_plugins/s3.py:407: error: "Path" has no attribute "_accessor"
+ src/bandersnatch_storage_plugins/s3.py:408: error: "Path" has no attribute "bucket"
+ src/bandersnatch_storage_plugins/s3.py:408: error: "Path" has no attribute "key"
+ src/bandersnatch_storage_plugins/s3.py:413: error: "Path" has no attribute "bucket"
+ src/bandersnatch_storage_plugins/s3.py:413: error: "Path" has no attribute "key"

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/persistence/models.py:90: error: Item "str" of "Optional[str]" has no attribute "isnot"
+ freqtrade/persistence/models.py:90: error: Item "None" of "Optional[str]" has no attribute "isnot"
+ freqtrade/persistence/models.py:888: error: Unsupported operand types for < ("datetime" and "None")
+ freqtrade/persistence/models.py:888: note: Left operand is of type "Optional[datetime]"
+ freqtrade/persistence/models.py:890: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:925: error: Item "str" of "Optional[str]" has no attribute "isnot"
+ freqtrade/persistence/models.py:925: error: Item "None" of "Optional[str]" has no attribute "isnot"
+ freqtrade/persistence/models.py:956: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:970: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:982: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:985: error: Unsupported operand types for <= ("datetime" and "None")
+ freqtrade/persistence/models.py:985: note: Left operand is of type "Optional[datetime]"
+ freqtrade/persistence/models.py:1015: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:1048: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:1081: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:1133: error: "bool" has no attribute "is_"
+ freqtrade/persistence/models.py:1133: error: Unsupported operand types for <= ("datetime" and "None")
+ freqtrade/persistence/models.py:1133: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:156: error: "int" has no attribute "in_"
+ freqtrade/rpc/rpc.py:284: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:285: error: Unsupported operand types for <= ("date" and "None")
+ freqtrade/rpc/rpc.py:285: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:286: error: Unsupported operand types for > ("date" and "None")
+ freqtrade/rpc/rpc.py:286: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:327: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:328: error: Unsupported operand types for <= ("date" and "None")
+ freqtrade/rpc/rpc.py:328: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:329: error: Unsupported operand types for > ("date" and "None")
+ freqtrade/rpc/rpc.py:329: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:369: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:370: error: Unsupported operand types for <= ("date" and "None")
+ freqtrade/rpc/rpc.py:370: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:371: error: Unsupported operand types for > ("date" and "None")
+ freqtrade/rpc/rpc.py:371: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:401: error: Item "datetime" of "Optional[datetime]" has no attribute "desc"
+ freqtrade/rpc/rpc.py:401: error: Item "None" of "Optional[datetime]" has no attribute "desc"
+ freqtrade/rpc/rpc.py:403: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:406: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:407: error: Item "datetime" of "Optional[datetime]" has no attribute "desc"
+ freqtrade/rpc/rpc.py:407: error: Item "None" of "Optional[datetime]" has no attribute "desc"
+ freqtrade/rpc/rpc.py:414: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:428: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:454: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:454: error: Unsupported operand types for <= ("datetime" and "None")
+ freqtrade/rpc/rpc.py:454: note: Left operand is of type "Optional[datetime]"
+ freqtrade/rpc/rpc.py:455: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:703: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:736: error: "bool" has no attribute "is_"
+ freqtrade/rpc/rpc.py:752: error: "bool" has no attribute "is_"
+ freqtrade/commands/list_commands.py:214: error: "int" has no attribute "in_"
+ freqtrade/freqtradebot.py:610: error: Incompatible types in assignment (expression has type "None", variable has type "str")
+ freqtrade/freqtradebot.py:856: error: Argument 2 to "update_trade_state" of "FreqtradeBot" has incompatible type "Optional[str]"; expected "str"
+ freqtrade/freqtradebot.py:913: error: No overload variant of "__sub__" of "datetime" matches argument type "None"
+ freqtrade/freqtradebot.py:913: note: Possible overload variants:
+ freqtrade/freqtradebot.py:913: note:     def __sub__(self, timedelta) -> datetime
+ freqtrade/freqtradebot.py:913: note:     def __sub__(self, datetime) -> timedelta
+ freqtrade/freqtradebot.py:913: note: Right operand is of type "Optional[datetime]"
+ freqtrade/freqtradebot.py:1033: error: Argument 1 to "cancel_order_with_result" of "Exchange" has incompatible type "Optional[str]"; expected "str"
+ freqtrade/freqtradebot.py:1059: error: Argument 2 to "update_trade_state" of "FreqtradeBot" has incompatible type "Optional[str]"; expected "str"
+ freqtrade/freqtradebot.py:1070: error: Argument 2 to "update_trade_state" of "FreqtradeBot" has incompatible type "Optional[str]"; expected "str"
+ freqtrade/freqtradebot.py:1092: error: Argument 1 to "cancel_order_with_result" of "Exchange" has incompatible type "Optional[str]"; expected "str"
+ freqtrade/freqtradebot.py:1111: error: Incompatible types in assignment (expression has type "None", variable has type "str")
+ freqtrade/freqtradebot.py:1234: error: Argument 2 to "update_trade_state" of "FreqtradeBot" has incompatible type "Optional[str]"; expected "str"

pandas (https://github.com/pandas-dev/pandas)
+ pandas/io/formats/format.py:559: error: Unsupported left operand type for + ("None")  [operator]
+ pandas/io/formats/format.py:559: note: Left operand is of type "Optional[str]"
+ pandas/tests/indexes/interval/test_base.py:19: error: "Type[Index]" has no attribute "from_breaks"  [attr-defined]
+ pandas/tests/indexes/categorical/test_category.py:25: error: Incompatible return value type (got "Index", expected "CategoricalIndex")  [return-value]

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

A lot of the diff comes from freqtrade, which for some reason, has code like this:

class LocalTrade:
  open_order_id: Optional[str] = None

class Trade(LocalTrade):
  open_order_id = Column(String(255))

Unfortunately, this is just contrary to the whole point of this PR.

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

I think this is an anti-feature. Maybe there's a middle ground to be had with something to do with Variance checking, but as it is, this breaks too many things.

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

class Storage:
  PATH_BACKEND: Type[pathlib.Path] = pathlib.Path

class SwiftStorage:
  PATH_BACKEND = SwiftPath

This is very reasonable code, and I want this to be reasonable. Something else needs to change.

@Riolku Riolku closed this Mar 3, 2022
@JelleZijlstra
Copy link
Member

In the freqtrade case, is Column a descriptor that returns str on access? Perhaps that's something we can handle better.

And the SwiftPath case seems like a real error if SwiftPath is not a subclass of Path.

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

I think that's sqlalchemy code, but accessing that property only returns a string when self is defined, because it's a Model (I think). SwiftPath is a subclass of Path.

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

By which I mean it only returns a string in the context of a non-static method or the like. That's what I recall from that library anyway.

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

I think the freqtrade case should throw, and honestly I'm rather confused as to why it doesn't, but I don't think this is the right way to fix it.

@JelleZijlstra
Copy link
Member

mypy isn't very strict about class/instance attributes working the same way in base and child classes I believe.

@Riolku
Copy link
Author

Riolku commented Mar 3, 2022

Do you think it should be? Would it be an improvement if it were more strict?

@JelleZijlstra
Copy link
Member

I'm not excited about it; it will technically fix some type safety issues but in practice I suspect it will likely mostly generate busywork. We'd probably have to come up with a generic way to say "instance-only attribute", which currently doesn't exist.

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.

Incorrect type inference in Derived class that was explicitly typed in Base
2 participants