-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Implement Indexed builder class #3
Conversation
@zonca The PR name should have a |
Forked repositories normally do not have secrets correctly configured See https://github.com/orgs/community/discussions/26409#discussioncomment-3251822
@zonca please make a PR to awkward if you are done with it. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ManasviGoyal has already approved this, and we only require one approval, but I looked it over also and approve it as well.
I'll merge it. Thanks, @zonca!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, that didn't set the "approve" checkmark. This should do it.
Nope. Oh well! |
Wait a minute, why can't I merge this? Oh! It's a PR into zonca/awkward. I see scikit-hep#2883 and will go there now. |
I'm closing this PR, let's refer to the one in the official repo |
I compare the implementation to a branch where I have just copy-pasted
IndexedOption
to more easily compare the differences between the 2 implementations.