Skip to content

CLN: Remove raise if missing only controlling the error message #41371

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 2 commits into from
May 7, 2021
Merged

CLN: Remove raise if missing only controlling the error message #41371

merged 2 commits into from
May 7, 2021

Conversation

phofl
Copy link
Member

@phofl phofl commented May 7, 2021

The removed tests don't make sense anymore, since all is printed now but without a fixed order

@phofl phofl added Clean Indexing Related to indexing on series/frames, not to indexes themselves labels May 7, 2021
jreback
jreback reviewed May 7, 2021
View reviewed changes
pandas/core/indexing.py
f"The following labels were missing: {not_found}. "
"See https://pandas.pydata.org/pandas-docs/stable/user_guide/indexing.html#deprecate-loc-reindex-listlike" # noqa:E501
)
not_found = list(set(key) - set(ax))
Copy link
Contributor

Choose a reason for hiding this comment

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

we may wan to limit the display here to max_seq_items (e.g. if you have a huge list), but could investigate if this is actually an issue. it can be potentially if it takes time to render and for example we are NOT raising this to the user (that's not true in this case, speaking more generally here).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this is a good point. Will try to investigate, if we can do this when we supress the error.

@jreback jreback added this to the 1.3 milestone May 7, 2021
@jreback jreback merged commit 67c9385 into pandas-dev:master May 7, 2021
@jreback
Copy link
Contributor

jreback commented May 7, 2021

thanks @phofl

@phofl phofl deleted the clean_raise_missing branch May 7, 2021 22:09
@TomAugspurger
Copy link
Contributor

Did this cause a performance regression?

https://pandas.pydata.org/speed/pandas/index.html#indexing.NumericSeriesIndexing.time_loc_slice?python=3.8&Cython=0.29.21&p-index_dtype=%3Cclass+%27pandas.core.indexes.numeric.UInt64Index%27%3E&p-index_structure=%27nonunique_monotonic_inc%27&commits=67c9385787c4d854b75a6f2c04fdf6886fdb1a06

@phofl
Copy link
Member Author

phofl commented May 11, 2021

This tick really looks similar to the one a few weeks earlier. But I will have a look.

Practically this should only impact cases where a KeyError is raised

@jreback
Copy link
Contributor

jreback commented May 12, 2021

@TomAugspurger if you wouldn't mind creating an issue about this regression

@TomAugspurger
Copy link
Contributor

Yeah, looks like a false alarm. The latest benchmarks are back to normal.

@jreback
Copy link
Contributor

jreback commented May 12, 2021

oh ok great!

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
@phofl @JulianWgs
CLN: Remove raise if missing only controlling the error message (pand…
aeea82e 
…as-dev#41371)
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

Assignees
No one assigned
Labels
Clean Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Milestone
1.3
Development

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