Skip to content

De-duplicate add_offset_array methods #19835

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 11 commits into from
Feb 24, 2018
Merged

Conversation

jbrockmendel
Copy link
Member

Combine DTI/TDI/PI add_offset_array and sub_offset_array methods.

Re-order __add__ and __sub__ if/else branches to put scalars up front.

This was going to include fixes for DTI/TDI with ndarray[timedelta64]/ndarray[datetime64], but the diff got big enough to split that off to the next one.

kwargs = {}
if not is_period_dtype(self):
kwargs['freq'] = 'infer'
return self.__class__(res_values, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

use .shallow_copy

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it to shallow_copy and it broke several tests, so just pushed with it changed back to self.__class__. For some reason DatetimeIndex._simple_new returns an ndarray if its input values have object-dtype, which is the case here.

Copy link
Contributor

Choose a reason for hiding this comment

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

so try using _shallow_copy_with_infer, we by definition never ever use self.__class__ in Index construction.

----------
other : Index, np.ndarray
object-dtype containing pd.DateOffset objects
op : {operator.add, operator.sub}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you assert on the operator

@jreback jreback added Datetime Datetime data dtype Timedelta Timedelta data type Period Period data type Clean labels Feb 22, 2018
@codecov
Copy link

codecov bot commented Feb 22, 2018

Codecov Report

Merging #19835 into master will increase coverage by <.01%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19835      +/-   ##
==========================================
+ Coverage   91.57%   91.58%   +<.01%     
==========================================
  Files         150      150              
  Lines       48903    48870      -33     
==========================================
- Hits        44785    44758      -27     
+ Misses       4118     4112       -6
Flag Coverage Δ
#multiple 89.96% <93.33%> (ø) ⬆️
#single 41.8% <30%> (+0.02%) ⬆️
Impacted Files Coverage Δ
pandas/core/indexes/datetimes.py 95.7% <ø> (+0.14%) ⬆️
pandas/core/indexes/period.py 93.04% <ø> (+0.16%) ⬆️
pandas/core/indexes/timedeltas.py 90.94% <66.66%> (+0.31%) ⬆️
pandas/core/indexes/datetimelike.py 97.14% <96.29%> (+0.24%) ⬆️
pandas/util/testing.py 83.64% <0%> (-0.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b585e3b...34a49a3. Read the comment docs.

@jbrockmendel jbrockmendel mentioned this pull request Feb 23, 2018
15 tasks
@jreback jreback added this to the 0.23.0 milestone Feb 24, 2018
@jreback jreback merged commit e8b80b1 into pandas-dev:master Feb 24, 2018
@jreback
Copy link
Contributor

jreback commented Feb 24, 2018

thanks!

harisbal pushed a commit to harisbal/pandas that referenced this pull request Feb 28, 2018
@jbrockmendel jbrockmendel deleted the intarrays branch June 22, 2018 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Datetime Datetime data dtype Period Period data type Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants