TYP: _config/config.py && core/{apply,construction}.py #30734
Conversation
There was a problem hiding this comment.
@MomIsBestFriend Thanks for the PR. generally lgtm ex Generics without type parameters, see #30539
ideally we want that list to shrink!
| @@ -211,7 +211,7 @@ def __getattr__(self, key: str): | |||
| else: | |||
| return _get_option(prefix) | |||
|
|
|||
| def __dir__(self): | |||
| def __dir__(self) -> Iterable[str]: | |||
There was a problem hiding this comment.
| def __dir__(self) -> Iterable[str]: | |
| def __dir__(self) -> List[str]: |
There was a problem hiding this comment.
I vaguely recall this coming up elsewhere. Actually looks like this is inconsistent in typeshed though stdlib matches what's here:
No strong preference on either, just adding for visibility
There was a problem hiding this comment.
I vaguely recall this coming up elsewhere.
I got the same feeling while annotating this.
So I looked hard in my previous PR until I found it.
#30411 (comment)
| @@ -818,7 +823,7 @@ def inner(x): | |||
| return inner | |||
|
|
|||
|
|
|||
| def is_nonnegative_int(value): | |||
| def is_nonnegative_int(value: Optional[int]) -> None: | |||
There was a problem hiding this comment.
strange that this returns None. I would expect a function named is_* to return True/False.
There was a problem hiding this comment.
I think OK to rename this check_nonnegative_int in another PR
pandas/_config/config.py
Outdated
| key: str, | ||
| defval: object, | ||
| doc: str = "", | ||
| validator: Optional[Union[Callable[..., None], Type[str]]] = None, |
There was a problem hiding this comment.
The Type[str] shouldn't be needed. changing the Callable to allow it to return a value fixes this. Also the validator should accept only a single argument.
| validator: Optional[Union[Callable[..., None], Type[str]]] = None, | |
| validator: Optional[Callable[[Any], Any]] = None, |
pandas/_config/config.py
Outdated
| @@ -729,7 +734,7 @@ def config_prefix(prefix): | |||
| global register_option, get_option, set_option, reset_option | |||
|
|
|||
| def wrap(func): | |||
| def inner(key, *args, **kwds): | |||
| def inner(key: str, *args, **kwds) -> Callable[[str], Any]: | |||
There was a problem hiding this comment.
I think this should be annotated using the pattern for decorators i.e annotate the outer function using a typevar and cast the return type of inner function, see https://mypy.readthedocs.io/en/latest/generics.html#declaring-decorators
no need to annotate the inner function
pandas/_config/config.py
Outdated
| defval: object, | ||
| doc: str = "", | ||
| validator: Optional[Union[Callable[..., None], Type[str]]] = None, | ||
| cb: Optional[Callable[[str], None]] = None, |
There was a problem hiding this comment.
Similar to validator, restricting the return type to None may not be desirable.
pandas/_config/config.py
Outdated
| @@ -751,7 +756,7 @@ def inner(key, *args, **kwds): | |||
| # arg in register_option | |||
|
|
|||
|
|
|||
| def is_type_factory(_type): | |||
| def is_type_factory(_type) -> Callable[..., None]: | |||
There was a problem hiding this comment.
the returned function accepts exactly one argument
| def is_type_factory(_type) -> Callable[..., None]: | |
| def is_type_factory(_type: Type[Any]) -> Callable[[Any], None]: |
pandas/_config/config.py
Outdated
| if type(x) != _type: | ||
| raise ValueError(f"Value must have type '{_type}'") | ||
|
|
||
| return inner | ||
|
|
||
|
|
||
| def is_instance_factory(_type): | ||
| def is_instance_factory(_type) -> Callable[..., None]: |
There was a problem hiding this comment.
the returned function accepts exactly one argument
| def is_instance_factory(_type) -> Callable[..., None]: | |
| def is_instance_factory(_type) -> Callable[[Any], None]: |
pandas/_config/config.py
Outdated
| if not isinstance(x, _type): | ||
| raise ValueError(f"Value must be an instance of {type_repr}") | ||
|
|
||
| return inner | ||
|
|
||
|
|
||
| def is_one_of_factory(legal_values): | ||
| def is_one_of_factory(legal_values) -> Callable[..., None]: |
There was a problem hiding this comment.
| def is_one_of_factory(legal_values) -> Callable[..., None]: | |
| def is_one_of_factory(legal_values) -> Callable[[Any] None]: |
|
Thanks @MomIsBestFriend |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff