Skip to content

REF: implement Categorical._box_func, make _box_func a method #36206

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 8, 2020
Merged

REF: implement Categorical._box_func, make _box_func a method #36206

merged 6 commits into from
Sep 8, 2020

Conversation

jbrockmendel
Copy link
Member

Defining _box_func on Categorical is to facilitate more code sharing with datetimelike. Also there are a bunch of places that do something similar but slightly different that I want to standardize within Categorical.

Changing _box_func from a property-lambda to a method on the datetimelike arrays gives a slight perf boost:

In [1]: import pandas as pd                                                     
In [2]: dti = pd.date_range("2016-01-01", periods=3)                 
           
In [3]: %timeit dti[1]                                                          
8.73 µs ± 50.8 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)  # <-- master
8.08 µs ± 119 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)   # <-- PR

@jreback jreback added the Refactor Internal refactoring of code label Sep 8, 2020
@jreback jreback added this to the 1.2 milestone Sep 8, 2020
jreback
jreback requested changes Sep 8, 2020
View reviewed changes
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.

small comment, ok if you want to do as a followon as well

pandas/core/arrays/timedeltas.py Outdated
@property
def _box_func(self):
return lambda x: Timedelta(x, unit="ns")
def _box_func(self, x):
Copy link
Contributor

Choose a reason for hiding this comment

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

ideally can you type the returns

@jbrockmendel
Copy link
Member Author

annotated+green

jreback
jreback approved these changes Sep 8, 2020
View reviewed changes
pandas/core/arrays/period.py
@property
def _box_func(self):
return lambda x: Period._from_ordinal(ordinal=x, freq=self.freq)
def _box_func(self, x) -> Union[Period, NaTType]:
Copy link
Contributor

Choose a reason for hiding this comment

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

should alias this kind of type at somepoint, maybe PeriodOrNaT (and the others as well)

@jreback jreback added the ExtensionArray Extending pandas with custom dtypes or arrays. label Sep 8, 2020
@jreback jreback merged commit 490a999 into pandas-dev:master Sep 8, 2020
@jbrockmendel jbrockmendel deleted the cat-box_func branch September 8, 2020 23:10
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@jbrockmendel
REF: implement Categorical._box_func, make _box_func a method (pandas…
7659f36 
…-dev#36206)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
ExtensionArray Extending pandas with custom dtypes or arrays. Refactor Internal refactoring of code
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.
2 participants
@jbrockmendel @jreback