-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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: misc fixes for numpy types 3 #36100
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
|
||
from pandas._libs import lib, writers as libwriters | ||
from pandas._libs.tslibs import timezones | ||
from pandas._typing import ArrayLike, FrameOrSeries, Label | ||
from pandas._typing import AnyArrayLikeUnion, ArrayLike, FrameOrSeries, Label | ||
from pandas.compat._optional import import_optional_dependency | ||
from pandas.compat.pickle_compat import patch_pickle | ||
from pandas.errors import PerformanceWarning | ||
|
@@ -5076,7 +5076,7 @@ def _dtype_to_kind(dtype_str: str) -> str: | |
return kind | ||
|
||
|
||
def _get_data_and_dtype_name(data: ArrayLike): | ||
def _get_data_and_dtype_name(data: AnyArrayLikeUnion): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be better to ensure we unwrap to avoid Index/Series? for my edification: i was under the impression that AnyArrayLike vs AnyArrayLikeUnion only mattered when we had more than one of them in a signature; i.e.
implies the return type matches the arg type, while using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in this case we change the type of data in the body. i.e assign to data and hence no preserving type of data. I've used ..Union to be consistent with FrameOrSeries. we could change ArrayLike and AnyArrayLike to be unions and ArrayLikeT and AnyArrayLikeT for type variables. or we could assign to another variable name in the body (simpler, but imo messy and less clear) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. im happy to follow your lead on this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @WillAyd wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think worth going down this path. We will likely need a Union type for variable type annotations anyway, since if we use ArrayLike we would get |
||
""" | ||
Convert the passed data into a storable form and a dtype string. | ||
""" | ||
|
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.
can you add a comment on when you should use the AnyArryLike TypeVar and when the Union (for readers of this code!)