Skip to content

REF: move Loc method that belongs on Index #31857

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
merged 4 commits into from
Feb 22, 2020
Merged

REF: move Loc method that belongs on Index #31857

merged 4 commits into from
Feb 22, 2020

Conversation

jbrockmendel
Copy link
Member

No description provided.

@WillAyd
Copy link
Member

WillAyd commented Feb 11, 2020

+/- 0 on this; I don't know that this is necessarily clearer as a index method but no major objections

@WillAyd WillAyd added the Index Related to the Index class or subclasses label Feb 11, 2020
@jbrockmendel
Copy link
Member Author

@WillAyd gentle ping

@jbrockmendel jbrockmendel added the Refactor Internal refactoring of code label Feb 19, 2020
@WillAyd
Copy link
Member

WillAyd commented Feb 20, 2020

I don't have a strong enough preference on this to go either way. @pandas-dev/pandas-core for other thoughts if a blocker

@jreback jreback added this to the 1.1 milestone Feb 20, 2020
@jreback
Copy link
Contributor

jreback commented Feb 20, 2020

will have a look soon

@jbrockmendel
Copy link
Member Author

this isnt a big deal, just a code smell that id like to get out of the way

@toobaz
Copy link
Member

toobaz commented Feb 20, 2020

+1: ideally if isinstance(labels, ABCMultiIndex): should never appear outside pandas/core/indexes/multi.py. Even clearer would be to remove the need for (non-multi) Index to support this method (but I didn't check how complicated it is - and this PR is certainly good per se).

@jreback jreback merged commit 10d10c6 into pandas-dev:master Feb 22, 2020
@jreback
Copy link
Contributor

jreback commented Feb 22, 2020

thanks, yeah getting Index/MI closer and closer but there will always be ovrrides.

@jbrockmendel jbrockmendel deleted the indexing-methods branch February 22, 2020 16:05
jreback
jreback reviewed Feb 22, 2020
View reviewed changes
pandas/core/indexes/multi.py
Translate any partial string timestamp matches in key, returning the
new key.

Only relevant for MultiIndex.
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove this comment at some point

@toobaz
Copy link
Member

toobaz commented Feb 22, 2020

thanks, yeah getting Index/MI closer and closer but there will always be ovrrides.

Sure! I was just saying that ideally overrides should be only in MI code.

roberthdevries pushed a commit to roberthdevries/pandas that referenced this pull request Mar 2, 2020
@jbrockmendel @roberthdevries
move method from LocationIndexer to Index (pandas-dev#31857)
33f599a
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

@jorisvandenbossche jorisvandenbossche jorisvandenbossche 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.
5 participants
@jbrockmendel @WillAyd @jreback @toobaz @jorisvandenbossche