Skip to content
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.

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

fix: protect Cat indexing from missing values #689

Merged
merged 3 commits into from
Jan 24, 2022

Conversation

andrzejnovak
Copy link
Member

@andrzejnovak andrzejnovak commented Jan 17, 2022

Category indexing will return len(index)+1 for missing index, which creates messy error messages down the line.
image

This should catch it correctly

image

Closes #387.

@github-actions github-actions bot added the needs changelog Might need a changelog entry label Jan 17, 2022
@henryiii
Copy link
Member

Related to #387. This will likely have a performance impact on accessing histograms, should be checked.

@andrzejnovak andrzejnovak force-pushed the indexfix branch 5 times, most recently from 53b4695 to 7953d2a Compare January 19, 2022 19:04
@andrzejnovak
Copy link
Member Author

@henryiii fixed the tests

andrzejnovak and others added 2 commits January 24, 2022 15:11
chore: make pre-commit happy

fix: handle errors in C++

style: pre-commit fixes

fix: ensure consistent passing

fix: C++17 makes noexcept part of the type system

style: pre-commit fixes

fix: too much noexcept
@henryiii
Copy link
Member

We are throwing a KeyError for a missing Category, and not throwing for Integer or other axes.

@github-actions github-actions bot removed the needs changelog Might need a changelog entry label Jan 24, 2022
@henryiii henryiii merged commit 99f25b9 into scikit-hep:develop Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Indexing categories
2 participants