TYP: Add annotation to assert_frame_equal and assert_series_equal (GH26302) #39504
Conversation
|
@WillAyd I looked into Mypy error when using |
| check_dtype: Union[bool, str] = True, | ||
| check_index_type: Union[bool, str] = "equiv", | ||
| check_series_type: bool = True, | ||
| check_less_precise: Union[bool, int] = no_default, |
There was a problem hiding this comment.
@simonjayhawkins how shall we type this?
There was a problem hiding this comment.
no_default is defined here https://github.com/pandas-dev/pandas/blob/master/pandas/_libs/lib.pyx#L2384
There was a problem hiding this comment.
revealed type is Any, so not yet an issue. Lets defer handling of this till it becomes an issue. i.e. when we merge the type stub from pyright for lib.pyx
| if check_freq and isinstance(left.index, (DatetimeIndex, TimedeltaIndex)): | ||
| if ( | ||
| check_freq | ||
| and isinstance(left.index, (DatetimeIndex, TimedeltaIndex)) |
There was a problem hiding this comment.
On L959 we use ridx.freq. Initially we didn't check if right.index was also of type DatetimeIndex or TimedeltaIndex and therefore mypy wasnt sure if the index had the freq attribute. This threw the following error
error: "Index" has no attribute "freq" [attr-defined]| ): | ||
| left: DataFrame, | ||
| right: DataFrame, | ||
| check_dtype: Union[bool, str] = True, |
There was a problem hiding this comment.
on L128 we call assert_frame_equal with check_dtype=check_dtype and the default value of check_dtype in that context is "equiv". If we don't type it is as Union[bool, str] we get the following error:
error: Argument "check_dtype" to "assert_frame_equal" has incompatible type "Union[bool, str]"; expected "bool" [arg-type]There was a problem hiding this comment.
could use Literal["equiv"] here instead of str for clarity.
|
@jreback could you have a look at my comments? Thanks! |
|
cc @simonjayhawkins if you can have a look. |
| left: Series, | ||
| right: Series, |
There was a problem hiding this comment.
see #29364 for a prior discussion on this. IIRC These asserters are intended to raise AssertionError, not TypeError if left of right is not of the correct type.
assert_series_equal is part of the public api and therefore types added here will be used when checking user code. This may lead to false positives, depending on how users are using this function.
probably best to leave left and right untyped for now.
|
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
|
closing as stale. @avinashpancham please ping if you want to pick this up again. |
Incremental typing PR for #26302