Skip to content

TYP: Implicit generic "Any" for builtins #30541

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

Merged
merged 13 commits into from
Dec 31, 2019

Conversation

simonjayhawkins
Copy link
Member

xref #30539

… Use "typing.List" and specify generic parameters
…e "typing.Dict" and specify generic parameters
…c "Any". Use "typing.List" and specify generic parameters
…c "Any". Use "typing.List" and specify generic parameters
@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Dec 29, 2019
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@@ -32,10 +33,12 @@
FilePathOrBuffer = Union[str, Path, IO[AnyStr]]

FrameOrSeries = TypeVar("FrameOrSeries", bound="NDFrame")
Scalar = Union[str, int, float, bool]
PythonScalar = Union[str, int, float, bool]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would move DatetimeLikeScalar closer to these (also would add some comments about various sections, e.g. these are Scalars )

@@ -270,7 +270,10 @@ def maybe_make_list(obj):
return obj


def maybe_iterable_to_list(obj: Union[Iterable, Any]) -> Union[list, Any]:
_T = TypeVar("_T")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should't we just do this in _typing (and just call it T)

@jreback jreback added this to the 1.0 milestone Dec 30, 2019
DatetimeLikeScalar = TypeVar("DatetimeLikeScalar", "Period", "Timestamp", "Timedelta")
PandasScalar = Union[DatetimeLikeScalar, "Interval"]
Scalar = Union[PythonScalar, PandasScalar]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter that this is a Union of a Union and a TypeVar? Maybe DatetimeLikeScalar should just be a Union?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we have the same issue as with FilePathOrBuffer, where IO is unbound in the Union and requires the addition of type parameters when using the alias.

and we don't really want that for Scalar. but we had Scalar as a union (for JSONSerializable) and DatetimeLikeScalar as a TypeVar.

Maybe DatetimeLikeScalar should just be a Union?

rather than change this, maybe should define PandasScalar = Union["Period", "Timestamp", "Timedelta", "Interval"] for now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea makes sense

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. @WillAyd over to you

@TomAugspurger
Copy link
Contributor

Is there a config setting to (dis)allow this per module? Or is it global?

@simonjayhawkins
Copy link
Member Author

Is there a config setting to (dis)allow this per module? Or is it global?

disallow_any_generics picks these up, see #30539, could be global or per module. To ONLY find these would require custom script/clever regex

@TomAugspurger
Copy link
Contributor

Gotcha. So we'll just need to watch out for these until #30539 is all done?

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. @TomAugsburger merge away whenever questions resolved

@TomAugspurger TomAugspurger merged commit 052ac7b into pandas-dev:master Dec 31, 2019
@TomAugspurger
Copy link
Contributor

Thanks!

@TomAugspurger
Copy link
Contributor

Grr sorry I messed up the merge. Did a rebase and merge instead of a squash. Sorry :/

@simonjayhawkins simonjayhawkins deleted the generics branch January 5, 2020 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants