Fix classmethod inference for TypeVars with PEP 696 defaults#1735
Fix classmethod inference for TypeVars with PEP 696 defaults#1735jacks0n wants to merge 1 commit intoDetachHead:mainfrom
Conversation
When calling a classmethod on an unparameterized generic class where a TypeVar has a PEP 696 default, pyright eagerly applies the default before attempting inference from arguments. This prevents valid inference and causes false positive errors. Changes: - Skip specializeWithDefaultTypeArgs for classmethods on classes with PEP 696 explicit defaults, deferring specialization so TypeVars remain free for inference from arguments - Extend constructorTypeVarScopeId propagation to classmethods (not just __init__/__new__) so unsolved TypeVars receive their defaults after argument matching - Add test file typeVarDefaultClass5.py covering factory classmethods, direct parameters, mixed TypeVars, and subclass inheritance
| // values for its type parameters. Skip this if we're suppressing the use | ||
| // of attribute access override, such as with dundered methods (like __call__). | ||
| if ( | ||
| isInstantiableClass(objectType) && | ||
| !objectType.priv.includeSubclasses && | ||
| objectType.shared.typeParams.length > 0 | ||
| ) { | ||
| // Skip this if we're suppressing the use of attribute access override, | ||
| // such as with dundered methods (like __call__). | ||
| if ((flags & MemberAccessFlags.SkipAttributeAccessOverride) === 0) { |
There was a problem hiding this comment.
why was the comment about __all__ moved? i think it makes more sense next to the condition it's talking about
| // For classmethods on classes with PEP 696 explicit defaults, | ||
| // defer specialization so TypeVars remain free for inference | ||
| // from arguments (defaults applied via constructorTypeVarScopeId). | ||
| if ( | ||
| !objectType.priv.typeArgs && | ||
| objectType.shared.typeParams.some((tp) => isTypeVar(tp) && tp.shared.isDefaultExplicit) |
There was a problem hiding this comment.
PEP 696: States that semantics are "unspecified" for cases where TypeVar defaults interact with inference
there's no #unspecified-semantics section on that page. was this hallucinated by an LLM? i believe this is the section where this behavior is mentioned.
since this change deliberately goes against the PEP, it should probably only be enabled with the enabledBasedFeatures flag
| // For classmethods on classes with PEP 696 explicit defaults, | ||
| // defer specialization so TypeVars remain free for inference | ||
| // from arguments (defaults applied via constructorTypeVarScopeId). | ||
| if ( | ||
| !objectType.priv.typeArgs && | ||
| objectType.shared.typeParams.some((tp) => isTypeVar(tp) && tp.shared.isDefaultExplicit) |
There was a problem hiding this comment.
also, while i'm more than happy to deviate from the PEPs when we disagree with them, i'm a bit reluctant to support this in particular because generics on classmethods are already unsafe. see #1088
i don't know if it's a good idea to introduce a change that makes it easier for users to rely on this potentially dangerous pattern, until that issue is resolved.
Code sample in basedpyright playground
from typing import reveal_type
class Foo[T = int]:
_value: T | None = None
@classmethod
def set_value(cls, value: T):
cls._value = value
@classmethod
def get_value(cls) -> T | None:
return cls._value
Foo.set_value("") # error. this change would prevent this error from being reported
reveal_type(Foo.get_value()) # basedpyright: `int | None`, runtime: `str`|
Diff from mypy_primer, showing the effect of this PR on open source code: steam.py (https://github.com/Gobot1234/steam.py)
- .../projects/steam.py/steam/client.py:1279:17 - error: Type of "_from_history" is unknown (reportUnknownMemberType)
- .../projects/steam.py/steam/client.py:1279:28 - error: Cannot access attribute "_from_history" for class "type[TradeOffer[ReceivingAssetT@TradeOffer, SendingAssetT@TradeOffer, UserT@TradeOffer]]"
- Could not bind method "_from_history" because "type[TradeOffer[Item[User], Item[ClientUser], User]]" is not assignable to parameter "cls"
- "type[TradeOffer[Item[User], Item[ClientUser], User]]" is not assignable to "type[TradeOffer[MovedItem[User], MovedItem[ClientUser], User]]"
- Type "type[TradeOffer[Item[User], Item[ClientUser], User]]" is not assignable to type "type[TradeOffer[MovedItem[User], MovedItem[ClientUser], User]]"
- Type parameter "ReceivingAssetT@TradeOffer" is covariant, but "Item[User]" is not a subtype of "MovedItem[User]"
- "Item[User]" is not assignable to "MovedItem[User]"
- Type parameter "SendingAssetT@TradeOffer" is covariant, but "Item[ClientUser]" is not a subtype of "MovedItem[ClientUser]"
- "Item[ClientUser]" is not assignable to "MovedItem[ClientUser]" (reportAttributeAccessIssue)
- .../projects/steam.py/steam/client.py:1283:35 - error: Type of "created_at" is unknown (reportUnknownMemberType)
- .../projects/steam.py/steam/client.py:1285:29 - error: Type of "timestamp" is unknown (reportUnknownMemberType)
- .../projects/steam.py/steam/client.py:1286:78 - error: Type of "user" is unknown (reportUnknownMemberType)
- .../projects/steam.py/steam/client.py:1286:78 - error: Type of "id64" is unknown (reportUnknownMemberType)
- .../projects/steam.py/steam/client.py:1288:29 - error: Type of "receiving" is unknown (reportUnknownMemberType)
- 8304 errors, 88 warnings, 0 notes
+ 8297 errors, 88 warnings, 0 notes
altair (https://github.com/vega/altair)
- .../projects/altair/altair/datasets/_loader.py:355:9 - warning: Type of "load" is partially unknown
- Type of "load" is "_Load[Unknown, LazyFrame[Any]]" (reportUnknownVariableType)
- .../projects/altair/altair/datasets/_loader.py:355:16 - warning: Type of "from_reader" is partially unknown
- Type of "from_reader" is "(reader: Reader[Unknown, LazyFrame[Any]], /) -> _Load[Unknown, LazyFrame[Any]]" (reportUnknownMemberType)
- 408 errors, 7626 warnings, 0 notes
+ 408 errors, 7624 warnings, 0 notes
trio (https://github.com/python-trio/trio)
- .../projects/trio/src/trio/testing/_raises_group.py:609:72 - error: Cannot assign to attribute "excinfo" for class "RaisesGroup[BaseExcT_co@RaisesGroup]*"
- "_ExceptionInfo[BaseException]" is not assignable to "_ExceptionInfo[BaseExceptionGroup[BaseExcT_co@RaisesGroup]]"
- Type parameter "MatchE@_ExceptionInfo" is covariant, but "BaseException" is not a subtype of "BaseExceptionGroup[BaseExcT_co@RaisesGroup]"
- "BaseException" is not assignable to "BaseExceptionGroup[BaseExcT_co@RaisesGroup]" (reportAttributeAccessIssue)
- 1601 errors, 13 warnings, 0 notes
+ 1600 errors, 13 warnings, 0 notes
spark (https://github.com/apache/spark)
- .../projects/spark/python/pyspark/ml/util.py:1090:16 - error: Type "RL@MLReadable" is not assignable to return type "RL@DefaultParamsReader"
- Type "RL@MLReadable" is not assignable to type "RL@DefaultParamsReader" (reportReturnType)
+ .../projects/spark/python/pyspark/ml/util.py:1089:9 - warning: Type of "instance" is unknown (reportUnknownVariableType)
+ .../projects/spark/python/pyspark/ml/util.py:1090:16 - warning: Return type is unknown (reportUnknownVariableType)
- 41014 errors, 144241 warnings, 0 notes
+ 41013 errors, 144243 warnings, 0 notes
check-jsonschema (https://github.com/python-jsonschema/check-jsonschema)
+ .../projects/check-jsonschema/src/check_jsonschema/schema_loader/resolver.py:24:9 - warning: Argument type is partially unknown
+ Argument corresponds to parameter "contents" in function "from_contents"
+ Argument type is "dict[Unknown, Unknown]" (reportUnknownArgumentType)
+ .../projects/check-jsonschema/src/check_jsonschema/schema_loader/resolver.py:97:13 - warning: Argument type is Any
+ Argument corresponds to parameter "contents" in function "from_contents" (reportAny)
- 106 errors, 592 warnings, 0 notes
+ 106 errors, 594 warnings, 0 notes
|
Summary
When calling a classmethod on an unparameterized generic class where a TypeVar has a PEP 696 default, pyright eagerly applies the default before attempting inference from arguments. This prevents valid inference and causes false positive errors.
mypy infers from classmethod arguments before falling back to defaults, which handles this pattern without requiring casts.
Example (psycopg pattern):
Spec Status
Given the spec is unspecified, this PR aligns basedpyright with mypy's more permissive behavior.
Changes:
specializeWithDefaultTypeArgsfor classmethods on classes with PEP 696 explicit defaults, deferring specialization so TypeVars remain free for inference from argumentsconstructorTypeVarScopeIdpropagation to classmethods (not just__init__/__new__) so unsolved TypeVars receive their defaults after argument matchingRelated Issues
Unknowntype arguments when using class factory which has access to bound and defaults microsoft/pyright#10688 - Factory classmethod with TypeVar defaults (open)Test plan
TypeVarDefaultClass5test covers key scenarios