Skip to content

Conversation

twoertwein
Copy link
Member

The two main changes are 1) consistently using property and 2) checking whether Period() returns NaT.

@twoertwein twoertwein marked this pull request as draft June 11, 2022 15:20
@twoertwein twoertwein marked this pull request as ready for review June 11, 2022 18:32
)
assert isinstance(vmin, Period)
assert isinstance(vmax, Period)
span = vmax.ordinal - vmin.ordinal + 1
Copy link
Member Author

Choose a reason for hiding this comment

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

Can safely assert as NaT has no attribute called ordinal

return Period(ordinal=int(x), freq=self.freq).strftime(fmt)
period = Period(ordinal=int(x), freq=self.freq)
assert isinstance(period, Period)
return period.strftime(fmt)
Copy link
Member Author

Choose a reason for hiding this comment

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

same here: NaT has no strftime

return Period(x[0], freq_str).to_timestamp().tz_localize(x.tz) == x[0]
period = Period(x[0], freq_str)
assert isinstance(period, Period)
return period.to_timestamp().tz_localize(x.tz) == x[0]
Copy link
Member Author

Choose a reason for hiding this comment

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

NaT has no to_timestamp

return "vertical"

@property
def _kind(self) -> Literal["line", "area", "hist", "kde", "box"]:
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed so that the sub-classes are compatible with LinePlot



def _get_period_alias(freq) -> str | None:
def _get_period_alias(freq: BaseOffset | str) -> str | None:
Copy link
Member

Choose a reason for hiding this comment

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

The pyi is typed as timedelta | BaseOffset | str and the docstring says to_offset takes timedeltas as well. Should that be included here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, yes timedelta should be allowed.

@mroeschke mroeschke added the Typing type annotations, mypy/pyright type checking label Jun 13, 2022
@mroeschke mroeschke added this to the 1.5 milestone Jun 13, 2022
@mroeschke mroeschke merged commit 696e9bd into pandas-dev:main Jun 14, 2022
yehoshuadimarsky pushed a commit to yehoshuadimarsky/pandas that referenced this pull request Jul 13, 2022
* TYP: plotting._matplotlib

* somehow super causes issues

* fix pickle issue: was accessing _kind on class

* and the last plotting file

* add timedelta
@twoertwein twoertwein deleted the _matplotlib branch September 21, 2022 15:29
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.

2 participants