Skip to content

REF: handle searchsorted casting within DatetimeLikeArray #30950

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
jreback merged 4 commits into from
Jan 18, 2020
Merged

REF: handle searchsorted casting within DatetimeLikeArray #30950

jreback merged 4 commits into from
Jan 18, 2020

Conversation

@jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel commented Jan 13, 2020

No description provided.

simonjayhawkins
simonjayhawkins requested changes Jan 13, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @jbrockmendel . I think changing exception types and error messages should have a whatsnew. also in the case of Exception type, I would regard a change as a breaking change. not sure what others think. Should we have a deprecation for this?

@simonjayhawkins simonjayhawkins added Refactor Internal refactoring of code Index Related to the Index class or subclasses labels Jan 13, 2020
@jbrockmendel
Copy link
Member Author

jbrockmendel commented Jan 13, 2020

I think changing exception types and error messages should have a whatsnew. also in the case of Exception type, I would regard a change as a breaking change. not sure what others think. Should we have a deprecation for this?

If it were on the Index subclasses I would agree, but for the DTA/TDA/PA (which we dont expect users to use directly) I think we're OK

@simonjayhawkins
Copy link
Member

simonjayhawkins commented Jan 13, 2020

If it were on the Index subclasses I would agree, but for the DTA/TDA/PA (which we dont expect users to use directly) I think we're OK

https://dev.pandas.io/docs/reference/api/pandas.array.html has examples on how to construct these.

@jbrockmendel jbrockmendel mentioned this pull request Jan 13, 2020
Closed
@jreback jreback added this to the 1.1 milestone Jan 15, 2020
jreback
jreback approved these changes Jan 15, 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.

I agree with @simonjayhawkins we should have a whatsnew note, but its a very minor change IMHO. otherwise lgtm. ping on green.

I think after this PR you can then share the common code in searchsorted (at least for Index).

simonjayhawkins
simonjayhawkins reviewed Jan 15, 2020
View reviewed changes
@jbrockmendel @simonjayhawkins
Update doc/source/whatsnew/v1.1.0.rst
24682dc 
Co-Authored-By: Simon Hawkins <simonjayhawkins@gmail.com>
@jbrockmendel
Copy link
Member Author

jbrockmendel commented Jan 15, 2020

ping

jreback
jreback reviewed Jan 18, 2020
View reviewed changes
pandas/core/indexes/timedeltas.py
"searchsorted requires compatible dtype or scalar, "
f"not {type(value).__name__}"
)
if isinstance(value, Index):
Copy link
Contributor

@jreback jreback Jan 18, 2020

Choose a reason for hiding this comment

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

can likely move this to the base class an use shared impl with Timedelta (and I guess with DTI?)

@jreback jreback merged commit 024925a into pandas-dev:master Jan 18, 2020
@jreback
Copy link
Contributor

jreback commented Jan 18, 2020

thanks

@jbrockmendel jbrockmendel deleted the ref-searchsorted branch January 20, 2020 00:26
@jbrockmendel jbrockmendel mentioned this pull request Jan 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simonjayhawkins simonjayhawkins simonjayhawkins requested changes

@jreback jreback jreback approved these changes

Assignees

No one assigned

Labels

Index Related to the Index class or subclasses Refactor Internal refactoring of code

Projects

None yet

Milestone

1.1

Development

Successfully merging this pull request may close these issues.

3 participants

@jbrockmendel @simonjayhawkins @jreback