Fix: broadcast during slice assign and align assign API - #19
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR tightens slice
assign()on the PHP side, validates broadcast compatibility in Rust before calling ndarray’s assign, drops the old reshape-to-destination behavior, and extends tests for broadcastedNDArrayassignment and clearer shape errors.Motivation and Context
ndarray’s assign already broadcasts the right-hand side to the destination view’s shape, but only when the shapes are broadcast-compatible; otherwise it can panic. The library had been rejecting or reshaping on the PHP side in ways that did not match that model. Aligning with ndarray means checking broadcast feasibility up front (using the same rules as elsewhere in the crate), returning a controlled shape error instead of risking a panic, and letting a single Rust path perform the assignment.
What’s Changed
assign()simplified to scalar fill or NDArray assign without private helpers, no element-count equality requirement when shapes broadcast, and no automatic reshape/copy to force matching shapes before assign.assign()(scalars andNDArrayonly), with documentation updated to describe broadcast behavior.Breaking Changes
assign()no longer accepts any type; callers must pass anNDArrayor a scalar (includingComplex). Previously that case threwInvalidArgumentException; it now fails with aTypeErrorfrom the stricter signature.