-
Notifications
You must be signed in to change notification settings - Fork 89
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
refactor: drop behavior
broadcasting
#2761
Conversation
It's no longer used, and we want to avoid merging it
9fe07a2
to
0c4ddd8
Compare
5a2a95a is preparation for another PR that made sense here. |
Codecov Report
Additional details and impacted files
|
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.
Now that string-broadcasting is built-in, no longer comes from ak.behavior
, the broadcast_and_apply
function no longer needs to ask for it as an argument, and that decouples n-ary → n-ary functions so that they don't have to mix behaviors
anymore.
You're right that this changes how ak.broadcast_arrays
works, and is therefore a technically breaking change, but it is extremely niche. Behaviors are generally thought of as global settings (Coffea, Vector), and arrays can carry their own behavior
just in case they're not. Encountering different behavior
on the inputs to ak.broadcast_arrays
would be that surprising, exceptional case. Depending on it would be very weird.
I think this can go in a patch release. And I'm happy with the changes as they are here; please go ahead and merge when you're ready!
The
behavior
for one array given to broadcasting functions no longer needs to be broadcast against the other array behaviors. Thus, let's remove that restriction.This would break users who introduce behaviors on array X, and expect
ak.broadcast_arrays
to merge them.