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.

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

searchsorted, repeat broken off from #24024 #24461

Merged
merged 8 commits into from
Dec 28, 2018
Merged

searchsorted, repeat broken off from #24024 #24461

merged 8 commits into from
Dec 28, 2018

Conversation

jbrockmendel
Copy link
Member

Along with fixes for check_compatible_with discussed in #24024, and additional tests for searchsorted and _check_compatible_with

@codecov
Copy link

codecov bot commented Dec 28, 2018 •
edited
Loading

Codecov Report

Merging #24461 into master will increase coverage by <.01%.
The diff coverage is 93.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24461      +/-   ##
==========================================
+ Coverage   92.31%   92.31%   +<.01%     
==========================================
  Files         165      165              
  Lines       52194    52252      +58     
==========================================
+ Hits        48182    48237      +55     
- Misses       4012     4015       +3
Flag Coverage Δ
#multiple 90.73% <93.1%> (ø) ⬆️
#single 42.95% <32.75%> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/datetimelike.py 97.53% <ø> (ø) ⬆️
pandas/core/arrays/period.py 98.47% <100%> (+0.04%) ⬆️
pandas/core/arrays/timedeltas.py 87.63% <100%> (+0.26%) ⬆️
pandas/core/arrays/datetimelike.py 95.65% <85.71%> (-0.44%) ⬇️
pandas/core/arrays/datetimes.py 98.25% <92.85%> (-0.14%) ⬇️
pandas/core/generic.py 96.62% <0%> (ø) ⬆️
pandas/util/testing.py 87.84% <0%> (+0.09%) ⬆️

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 f4f37f4...9873c12. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 28, 2018

Codecov Report

Merging #24461 into master will decrease coverage by 49.3%.
The diff coverage is 32.78%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24461       +/-   ##
===========================================
- Coverage    92.3%   42.99%   -49.31%     
===========================================
  Files         163      163               
  Lines       51969    52028       +59     
===========================================
- Hits        47968    22372    -25596     
- Misses       4001    29656    +25655
Flag Coverage Δ
#multiple ?
#single 42.99% <32.78%> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/datetimelike.py 54.59% <ø> (-43.11%) ⬇️
pandas/core/arrays/period.py 37.47% <23.07%> (-61.02%) ⬇️
pandas/core/arrays/datetimes.py 64.99% <28.57%> (-32.62%) ⬇️
pandas/core/arrays/datetimelike.py 47.85% <36.36%> (-48.08%) ⬇️
pandas/core/arrays/timedeltas.py 40.34% <41.66%> (-46.58%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
... and 126 more

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 aa1549f...9a0bb4a. Read the comment docs.

@TomAugspurger TomAugspurger added Datetime Datetime data dtype ExtensionArray Extending pandas with custom dtypes or arrays. labels Dec 28, 2018
@jbrockmendel jbrockmendel mentioned this pull request Dec 28, 2018
Merged
12 tasks
@jbrockmendel
Copy link
Member Author

updated per comments

TomAugspurger
TomAugspurger reviewed Dec 28, 2018
View reviewed changes
pandas/core/arrays/datetimelike.py Outdated
# These are not part of the EA API, but we implement them because
# pandas assumes they're there.

def searchsorted(self, v, side='left', sorter=None):
Copy link
Contributor

@TomAugspurger TomAugspurger Dec 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Just to be clear, the name for the positional argument hasn't been finalized yet. We're figuring out the relative value of matching NumPy vs. matching the rest of pandas here.

Copy link
Contributor

Choose a reason for hiding this comment

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

well, as I have said multiple times, we are already using value in pandas, so -1 unless its that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently Jeff has decided :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Will revert

TomAugspurger
TomAugspurger reviewed Dec 28, 2018
View reviewed changes
pandas/core/arrays/datetimelike.py Outdated

def _check_compatible_with(self, other):
# type: (Union[Period, Timestamp, Timedelta, NaTType]) -> None
# TODO: Scalar, array, or both?
Copy link
Contributor

Choose a reason for hiding this comment

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

This TODO can be removed I think. We've finalized the type IMO.

But that can be done on merge if we don't have any other commits to push.

@jreback
Copy link
Contributor

jreback commented Dec 28, 2018

merging this and closing #24350 ?

@TomAugspurger
Copy link
Contributor

#24350 implements it for ExtensionArray. This does it for datetimelike, so both will be needed I think.

@jreback
Copy link
Contributor

jreback commented Dec 28, 2018

okie dokie; this needs a merge master

@jreback jreback added this to the 0.24.0 milestone Dec 28, 2018
@jbrockmendel
Copy link
Member Author

rebased

@jreback
Copy link
Contributor

jreback commented Dec 28, 2018

merged #24350 but looks like this is clean. ping on green.

@jbrockmendel
Copy link
Member Author

Ping

@jreback jreback merged commit 985a73b into pandas-dev:master Dec 28, 2018
@jreback
Copy link
Contributor

jreback commented Dec 28, 2018

thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the eadata3 branch December 28, 2018 23:27
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@jbrockmendel @Pingviinituutti
searchsorted, repeat broken off from pandas-dev#24024 (pandas-dev#24461)
5a9816f
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@jbrockmendel @Pingviinituutti
searchsorted, repeat broken off from pandas-dev#24024 (pandas-dev#24461)
e239ab3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback left review comments

@TomAugspurger TomAugspurger TomAugspurger approved these changes

Assignees
No one assigned
Labels
Datetime Datetime data dtype ExtensionArray Extending pandas with custom dtypes or arrays.
Projects
None yet
Milestone
0.24.0
Development

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