Retrofit assert_type for Aggregate and Callable #935
Conversation
There was a problem hiding this comment.
I've only worked on aggregate.py and callable.py, but found some areas that need quick-fixings.
I think it'd make more sense to include these quick-fixes in the same PR, as the tests are supposed to check exactly the behaviors these quick-fixes are supposed to affect.
@ZipFile let me know what you think.
| | ||
| | ||
| provider1_new_non_string_keys: providers.Aggregate[str] = providers.Aggregate( | ||
| # TODO: Change providers.Aggregate to accept Mapping? Then remove explicit typing here |
There was a problem hiding this comment.
Current stub:
class Aggregate(Provider[T]): def __init__( self, provider_dict: Optional[_Dict[Any, Provider[T]]] = None, **provider_kwargs: Provider[T], ): ...I think we should accept collections.abc.Mapping here
class Aggregate(Provider[T]): def __init__( self, provider_dict: Optional[Mapping[Any, Provider[T]]] = None, **provider_kwargs: Provider[T], ): ...Because Dict is invariant in both key and value, so providers.Aggregate cannot infer its own typevar without explicit typing
provider1_new_non_string_keys = providers.Aggregate( {Cat: providers.Object("str")}, ) # This raises mypy error| args4 = provider4.args | ||
| kwargs4 = provider4.kwargs | ||
| assert_type(args4, Tuple[Any]) | ||
| # TODO: Change Callable.kwargs to Dict[str, Any]? Then adjust test back to Dict[str, Any] |
There was a problem hiding this comment.
The stub actually declares provider4.kwargs to return Dict[Any, Any], not Dict[str, Any] like the previous test probably intended to check.
But I think we can safely change it to return Dict[str, Any] in the stub.
| attr_getter5 = provider5.provided.attr | ||
| item_getter5 = provider5.provided["item"] | ||
| method_caller5 = provider5.provided.method.call(123, arg=324) | ||
| # TODO: Remove explicit typing of Provider.provided return type |
There was a problem hiding this comment.
The stub is this
@property def provided(self) -> ProvidedInstance[T]: ...But ProvidedInstance is not declared to be generic
class ProvidedInstance(Provider, ProvidedInstanceFluentInterface): def __init__(self, provides: Optional[Provider] = None) -> None: ...I think we can make the quick fix to
@property def provided(self) -> ProvidedInstance: ...| # Test 10: to check the .provides | ||
| provider10 = providers.Callable(Cat) | ||
| provides10: Optional[Callable[..., Cat]] = provider10.provides | ||
| assert provides10 is Cat |
There was a problem hiding this comment.
I think these assert statements are not doing anything
| | ||
| # Test 12: to check string imports | ||
| provider12: providers.Callable[Dict[Any, Any]] = providers.Callable("builtins.dict") | ||
| # TODO: Use T_Any as Callable typevar? Then remove type-ignore |
There was a problem hiding this comment.
Can't infer typevar without explicit typing, mypy raises error here, but we can change it to typevar with default Any to fix it
2 participants
Partially addresses #934