BUG: PeriodIndex.get_loc with mismatched freq

[jbrockmendel]

• closes #xxxx
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

[simonjayhawkins]

is there an issue for this, does it need a release note?

[jreback]

lgtm ping with a whatsnew note

[simonjayhawkins]

idx.get_loc(datetime.datetime(2000, 1, 2, 1, 0), "pad"), idx.get_loc(pd.Timestamp("2000-01-02 01:00:00"), "pad") and idx.get_loc('2000-01-02 00:00', "pad") still return 1. why do we need to disallow a mismatched Period object?

[jbrockmendel]

i dont think there is a user-facing change bc __getitem__ goes through engine.get_loc where this needs to be addressed separately

[simonjayhawkins]

Index.get_loc is public? so is a user-facing change? maybe should raise ValueError with appropriate message so it's more obvious when things fail. Raising a KeyError for a method that is supposed to return a value even when an exact match is not found seems counterintuitive.

[jbrockmendel]

Index.get_loc is public? so is a user-facing change?

get_loc is in that weird space where it isn't really intended for users, but it is called from outside self, so it would be a code smell to make it private

I'll add a whatsnew note to be on the safe side

maybe should raise ValueError with appropriate message so it's more obvious when things fail. Raising a KeyError for a method that is supposed to return a value even when an exact match is not found seems counterintuitive

get_loc is pretty consistent about raising KeyError IIRC

[simonjayhawkins]

maybe should raise ValueError with appropriate message so it's more obvious when things fail. Raising a KeyError for a method that is supposed to return a value even when an exact match is not found seems counterintuitive

get_loc is pretty consistent about raising KeyError IIRC

seems reasonable, my concern is that we are changing tested behavior without a deprecation and therefore it could break users code, so that break should be more obvious.

maybe add a warning or deprecate this behavior instead.

[simonjayhawkins]

Index.get_loc is public? so is a user-facing change?

get_loc is in that weird space where it isn't really intended for users, but it is called from outside self, so it would be a code smell to make it private

nothing in the api to suggest to the user that this method should not be used https://pandas.pydata.org/pandas-docs/dev/reference/api/pandas.Index.get_loc.html

[jbrockmendel]

nothing in the api to suggest to the user that this method should not be used

OK. I added a whatsnew note.

[jreback]

thanks @jbrockmendel

[jorisvandenbossche]

This seems to have caused slowdowns in several of the Period indexing benchmarks: see https://pandas.pydata.org/speed/pandas/#regressions?sort=1&dir=desc, scrolldown to "2021-05-28 17:58"

[jorisvandenbossche]

I opened #42247 to keep track of this regression.