-
Notifications
You must be signed in to change notification settings - Fork 51
Feature: Implement RESHAPE() in TDI #3012
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
base: alpha
Are you sure you want to change the base?
Conversation
e1e0f08 to
f32a353
Compare
joshStillerman
left a comment
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.
it points at the same memory - good
Do you check if it is the right size for the dimensions ?
|
|
||
| new_array.aflags.coeff = 1; | ||
| new_array.dimct = dimct; | ||
| new_array.a0 = new_array.pointer; |
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.
it points at the same memory - good
Do you check if it is the right size for the dimensions ?
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.
Yep! That's done up on line 77, I compare the original count to the product of the shape, which should be the new size
f32a353 to
4f8a763
Compare
This builtin was never implemented, but the opcode was allocated and it existed in the original documentation. The similarity to numpy's reshape function means users might actually want to use this, and throwing a terse error is less than helpful. This implements RESHAPE() with only the first two arguments of SOURCE and SHAPE. The optional arguments of PAD and ORDER described in the original documetnation are not implemented, and won't be included in the new documentation for this function. ``` TDI> reshape(1:6, [2, 3]) [[1,2], [3,4], [5,6]] TDI> reshape(1:6, [3, 2]) [[1,2,3], [4,5,6]] TDI> reshape(1:8, [2, 2, 2]) [[[1,2], [3,4]], [[5,6], [7,8]]] TDI> reshape(reshape(1:8, [2, 4]), [8]) [1,2,3,4,5,6,7,8] TDI> reshape([[1,2], [3,4]], [1, 4]) [[1], [2], [3], [4]] ```
4f8a763 to
9bf426b
Compare
This builtin was never implemented, but the opcode was allocated and it existed in the original documentation. The similarity to numpy's reshape function means users might actually want to use this, and throwing a terse error is less than helpful.
This implements RESHAPE() with only the first two arguments of SOURCE and SHAPE. The optional arguments of PAD and ORDER described in the original documetnation are not implemented, and won't be included in the new documentation for this function.