-
Notifications
You must be signed in to change notification settings - Fork 81
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: string broadcasting #2552
fix: string broadcasting #2552
Conversation
This PR is done! We will possibly need to revisit this again in future; broadcasting is a fiddly thing, but it's definitely an improvement, and removes some |
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.
These are the right tests to make work. I had some comments on the implementation until I noticed that I was wrong, and deleted them.
It all looks good! Go ahead and merge when you're ready.
Codecov Report
Additional details and impacted files
|
There are still some outstanding problems with string broadcasting, that pertain to parameter replacement in
broadcast_and_apply
.The main cases are covered in the tests as
xfail
.This PR fixes the broadcasting rules for parameters, with the following changes: