TYP: io.json._json, util._decorators #36903
Conversation
|
@jbrockmendel I'll review this shortly. FYI I've started to create a branch that just tracks the outstanding mypy errors for untyped defs. Might help in resolving some more of these (and finding the easy to fix modules). master...simonjayhawkins:untyped-defs |
Neat. I'm currently working on a branch that defines the relevant dunder methods on Index/Series/DataFrame directly instead of pinning them, which should address a bunch of these. |
cool. There is quite a lot of the errors due to mypy 0.74 typing self. For now, to get the modules off the exclude list can use setattr since mypy doesn't complain about that. The other issue with self being typed is the mixins. |
There was a problem hiding this comment.
Thanks @jbrockmendel generally lgtm, just the two ignores (are these real issues?)
pandas/util/_decorators.py
Outdated
| @@ -278,7 +278,8 @@ def decorate(func): | |||
| allow_args = allowed_args | |||
| else: | |||
| spec = inspect.getfullargspec(func) | |||
| allow_args = spec.args[: -len(spec.defaults)] | |||
| # mypy doesn't know that spec.defaults is Sized | |||
| allow_args = spec.args[: -len(spec.defaults)] # type:ignore[arg-type] | |||
There was a problem hiding this comment.
the mypy error is pandas\util\_decorators.py:282: error: Argument 1 to "len" has incompatible type "Optional[Tuple[Any, ...]]"; expected "Sized" [arg-type]
(side note: when ignoring I prefer to see the mypy error as a comment, I think it helps when browsing the code for type: ignores to remove as well as during review)
indeed spec.defaults could be None.
>>> def func(a, b, c):
... pass
...
>>>
>>> inspect.getfullargspec(func)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
>>>
does allowed_args ensure defaults is not None here?
if so, would a assert spec.defaults is not None be more appropriate that an ignore? (with a comment as to why is cannot be None)
| @@ -848,6 +848,8 @@ def __next__(self): | |||
|
|
|||
|
|
|||
| class Parser: | |||
| _split_keys: Tuple[str, ...] | |||
| _default_orient: str | |||
There was a problem hiding this comment.
can do it here or as follow-on, but we should do the same for Writer and remove the ignore there.
| # gets multiple values for keyword argument "dtype_if_empty | ||
| self.obj = create_series_with_explicit_dtype( | ||
| *data, dtype_if_empty=object | ||
| ) # type:ignore[misc] |
There was a problem hiding this comment.
the default for dtype_if_empty is object, so do we need to pass dtype_if_empty.
if data has more than six items, this would indeed raise.
>>> def func(a=None, b=None, c=object):
... print(c)
...
>>> data = [1, 2]
>>> func(*data, c=20)
20
>>>
>>> data = [1, 2, 3]
>>> func(*data, c=20)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: func() got multiple values for argument 'c'
>>>
do we know what data contains?
There was a problem hiding this comment.
not off the top of my head. @WillAyd any idea?
There was a problem hiding this comment.
Not sure. Assuming this path comes from the numpy keyword we've deprecated that anyway, so not worth investing a ton of time in #30636
There was a problem hiding this comment.
Thanks @jbrockmendel for the updates.
I would be happy to merge this and tackle the ignore in a follow-on in order to get untyped defs checked from this point on.
Since you removed a type:ignore, the net is none added!
|
Thanks @jbrockmendel |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff