Skip to content

Conversation

@jbrockmendel
Copy link
Member

I'm pretty sure we can share more code here, but PeriodEngine needs some work first.

@jbrockmendel jbrockmendel added Bug Indexing Related to indexing on series/frames, not to indexes themselves labels Mar 12, 2020
@jreback jreback added this to the 1.1 milestone Mar 14, 2020
@jreback
Copy link
Contributor

jreback commented Mar 14, 2020

lgtm. needs a rebase. is this user visible at a high level? e.g. in .loc or someting?

@jbrockmendel
Copy link
Member Author

is this user visible at a high level? e.g. in .loc or someting?

Yes, will update the test introduced in #30446

@jreback
Copy link
Contributor

jreback commented Mar 14, 2020

also needs a rebase

@jbrockmendel
Copy link
Member Author

hmm this should probably raise a KeyError instead of a TypeError?

df = pd.DataFrame(
    np.random.randn(5, 3),
    columns=["a", "b", "c"],
    index=pd.date_range("2012", freq="H", periods=5),
)

df = df.iloc[[0, 2, 2, 3]].copy() 

dti = df.index
tdi = pd.TimedeltaIndex(dti.asi8)  # matching i8 values

with pytest.raises(TypeError, match="is not convertible to datetime"):
    df.loc[tdi]

with pytest.raises(TypeError, match="is not convertible to datetime"):
    df["a"].loc[tdi]

@jreback
Copy link
Contributor

jreback commented Mar 14, 2020

hmm this should probably raise a KeyError instead of a TypeError?

df = pd.DataFrame(
    np.random.randn(5, 3),
    columns=["a", "b", "c"],
    index=pd.date_range("2012", freq="H", periods=5),
)

df = df.iloc[[0, 2, 2, 3]].copy() 

dti = df.index
tdi = pd.TimedeltaIndex(dti.asi8)  # matching i8 values

with pytest.raises(TypeError, match="is not convertible to datetime"):
    df.loc[tdi]

with pytest.raises(TypeError, match="is not convertible to datetime"):
    df["a"].loc[tdi]

hmm, yep sounds right

with pytest.raises(KeyError, match=msg):
df.loc[tdi]

with pytest.raises(KeyError, match=msg):
Copy link
Contributor

Choose a reason for hiding this comment

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

i would test the reverse as well (e.g. TDI is the index with a DTI as indexer). This should also raise for Period, I think? (these can be handled as followons)

@jreback jreback merged commit 5081748 into pandas-dev:master Mar 17, 2020
@jreback
Copy link
Contributor

jreback commented Mar 17, 2020

thanks

SeeminSyed pushed a commit to CSCD01-team01/pandas that referenced this pull request Mar 22, 2020
@jbrockmendel jbrockmendel deleted the bug-is_all_dates branch March 23, 2020 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Indexing Related to indexing on series/frames, not to indexes themselves

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants