Skip to content
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

[ENH] Move intersection functions for DatetimeIndex and TimedeltaIndex to Datetimelike and added new tests #25913

Merged
merged 17 commits into from
Apr 30, 2019

Conversation

fdroessler
Copy link
Contributor

Because DatetimeIndex and TimedeltaIndex intersections are similar, I have moved the intersection function of both classes up into Datetimelike after discussions with @reidy-p @jorisvandenbossche. At the same time for PeriodIndex I have used Index.intersection instead. Additionally I have added some tests to TimedeltaIndex recycling some tests from DatetimeIndex.

Re-open from #25121

@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

Merging #25913 into master will increase coverage by <.01%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25913      +/-   ##
==========================================
+ Coverage   91.56%   91.56%   +<.01%     
==========================================
  Files         175      175              
  Lines       52775    52760      -15     
==========================================
- Hits        48324    48312      -12     
+ Misses       4451     4448       -3
Flag Coverage Δ
#multiple 90.13% <93.75%> (ø) ⬆️
#single 41.84% <43.75%> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/period.py 92.12% <100%> (+0.05%) ⬆️
pandas/core/indexes/datetimes.py 96.86% <100%> (+0.45%) ⬆️
pandas/core/indexes/timedeltas.py 91.09% <100%> (+0.82%) ⬆️
pandas/core/indexes/datetimelike.py 97.89% <92.3%> (-0.64%) ⬇️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.56% <0%> (+0.1%) ⬆️

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 31b4019...05c2916. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

Merging #25913 into master will increase coverage by <.01%.
The diff coverage is 95.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25913      +/-   ##
==========================================
+ Coverage   91.97%   91.97%   +<.01%     
==========================================
  Files         175      175              
  Lines       52368    52349      -19     
==========================================
- Hits        48164    48148      -16     
+ Misses       4204     4201       -3
Flag Coverage Δ
#multiple 90.53% <95.45%> (ø) ⬆️
#single 40.72% <52.27%> (-0.12%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/period.py 92.1% <100%> (+0.05%) ⬆️
pandas/core/indexes/datetimes.py 96.85% <100%> (+0.45%) ⬆️
pandas/core/indexes/timedeltas.py 91.59% <100%> (+0.86%) ⬆️
pandas/core/indexes/datetimelike.py 98.13% <94.28%> (-0.4%) ⬇️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 96.9% <0%> (-0.12%) ⬇️

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 9feb3ad...0d7205d. Read the comment docs.

@fdroessler
Copy link
Contributor Author

@gfyoung not sure if I should mark the comments as resolved or if you do it once you had a look at it. Please let me know what the general practice is on those so I know for the future.

doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved
doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved
doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved
pandas/core/indexes/datetimelike.py Outdated Show resolved Hide resolved
pandas/core/indexes/datetimelike.py Outdated Show resolved Hide resolved
pandas/core/indexes/datetimelike.py Show resolved Hide resolved
pandas/core/indexes/timedeltas.py Show resolved Hide resolved
pandas/tests/indexes/timedeltas/test_setops.py Outdated Show resolved Hide resolved
pandas/tests/indexes/timedeltas/test_setops.py Outdated Show resolved Hide resolved
pandas/core/indexes/timedeltas.py Outdated Show resolved Hide resolved
pandas/tests/indexes/timedeltas/test_setops.py Outdated Show resolved Hide resolved
@fdroessler
Copy link
Contributor Author

@jreback Is there anything else? Do you want me to create a new issue re restructuring the tests?

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 comments, yes pls file an issue to consolidate set_op tests across indexes

pandas/tests/indexes/timedeltas/test_setops.py Outdated Show resolved Hide resolved
pandas/tests/indexes/timedeltas/test_setops.py Outdated Show resolved Hide resolved
pandas/tests/indexes/timedeltas/test_setops.py Outdated Show resolved Hide resolved
@jreback jreback added this to the 0.25.0 milestone Apr 12, 2019
@jreback
Copy link
Contributor

jreback commented Apr 12, 2019

@TomAugspurger if you could have a look

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Apr 12, 2019 via email

@jreback
Copy link
Contributor

jreback commented Apr 20, 2019

@fdroessler can you merge master and will have a look.

@fdroessler
Copy link
Contributor Author

@jreback sure, done

doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved
pandas/core/indexes/timedeltas.py Outdated Show resolved Hide resolved
pandas/core/indexes/datetimes.py Show resolved Hide resolved
@jreback
Copy link
Contributor

jreback commented Apr 28, 2019

can you merge master

@fdroessler
Copy link
Contributor Author

Done @jreback

@jreback jreback merged commit 34da8be into pandas-dev:master Apr 30, 2019
@jreback
Copy link
Contributor

jreback commented Apr 30, 2019

thanks @fdroessler very nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TimedeltaIndex.intersection has no sort option.
5 participants