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

commented Mar 28, 2019

  • closes #24966
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

@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.

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 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/tests/indexes/timedeltas/test_setops.py

fdroessler added some commits Apr 2, 2019

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 pandas/tests/indexes/timedeltas/test_setops.py
Show resolved Hide resolved pandas/tests/indexes/timedeltas/test_setops.py
@fdroessler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

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

@jreback
Copy link
Contributor

left a comment

small comments, yes pls file an issue to consolidate set_op tests across indexes

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/tests/indexes/timedeltas/test_setops.py Outdated

@jreback jreback added this to the 0.25.0 milestone Apr 12, 2019

@jreback

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

@TomAugspurger if you could have a look

@TomAugspurger

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

@jreback

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2019

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

@fdroessler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2019

@jreback sure, done

Show resolved Hide resolved 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
@jreback

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2019

can you merge master

@fdroessler

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

Done @jreback

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

11 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
pandas-dev.pandas Build #20190429.23 succeeded
Details
pandas-dev.pandas (Checks_and_doc) Checks_and_doc succeeded
Details
pandas-dev.pandas (Linux py35_compat) Linux py35_compat succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow) Linux py36_locale_slow succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow_old_np) Linux py36_locale_slow_old_np succeeded
Details
pandas-dev.pandas (Linux py37_locale) Linux py37_locale succeeded
Details
pandas-dev.pandas (Linux py37_np_dev) Linux py37_np_dev succeeded
Details
pandas-dev.pandas (Windows py36_np15) Windows py36_np15 succeeded
Details
pandas-dev.pandas (Windows py37_np141) Windows py37_np141 succeeded
Details
pandas-dev.pandas (macOS py35_macos) macOS py35_macos succeeded
Details
@jreback

This comment has been minimized.

Copy link
Contributor

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
Projects
None yet
5 participants
You can’t perform that action at this time.