-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
TYP: fix ignores #40389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TYP: fix ignores #40389
Conversation
simonjayhawkins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jbrockmendel will push a merge master since there are still mypy errors here to resolve.
| vals = vals.to_numpy( # type: ignore[attr-defined] | ||
| dtype=float, na_value=np.nan | ||
| ) | ||
| if isinstance(vals, ExtensionArray): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function signature has vals: np.ndarray. also needs updating
| ) | ||
|
|
||
| inference = None | ||
| inference: Optional[np.dtype] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the return type is Tuple[np.ndarray, Optional[Type]] in the function signature. so that can also now be narrowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was needed anyway for mypy fixup
pandas/core/reshape/reshape.py
Outdated
| comp_index = ensure_platform_int(comp_index) | ||
| stride = self.index.levshape[self.level] + self.lift | ||
| self.full_shape = ngroups, stride | ||
| self.full_shape = ngroups, int(stride) # int() for mypy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably don't want to be casting numpy integer types to python ints just to appease mypy. We probably need an int-like alias to use where we currently use int in type annotations.
I think better to leave this unchanged until we've had that discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, will revert
| def _reorder_for_extension_array_stack(arr, n_rows: int, n_columns: int): | ||
| def _reorder_for_extension_array_stack( | ||
| arr: ExtensionArray, n_rows: int, n_columns: int | ||
| ) -> ExtensionArray: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably should be a typevar. but also ok to leave as is for now if not needed as this is internal
| sort_remaining: bool, | ||
| key: IndexKeyFunc, | ||
| ) -> Optional[np.array]: # type: ignore[valid-type] | ||
| ) -> Optional[np.ndarray]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you found the bonus low hanger! 😄
pandas/core/tools/datetimes.py
Outdated
| # because it expects an ndarray argument | ||
| if isinstance(arg, IntegerArray): | ||
| result = arg.astype(f"datetime64[{unit}]") | ||
| res = arg.astype(f"datetime64[{unit}]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: res-> arr (I think once the type is added for arg in the signature, we will get an Incompatible types in assignment in arg = getattr(arg, "_values", arg) anyway.
| # error: Incompatible types in assignment (expression has type "TimedeltaIndex", | ||
| # variable has type "ndarray") | ||
| value = TimedeltaIndex(value, unit="ns", name=name) # type: ignore[assignment] | ||
| value = TimedeltaIndex(td64arr, unit="ns", name=name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also return TimedeltaIndex ... without the intermediate assignment, but the td64arr naming is also an improvement anyway.
simonjayhawkins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jbrockmendel for the updates. lgtm pending green
i get a bunch of noise in my local mypy output, so will have to see what the CI says