Add the transpose function for SimpleArray#509
Conversation
ThreeMonth03
force-pushed
the
issue506
branch
3 times, most recently
from
55a3613 to
292506b
Compare
| self.assertEqual(sarr_clone[3], 2.0) # should be the original value | ||
|
|
||
| def test_SimpleArray_transpose(self): | ||
| def check(sarr, ndarr): |
There was a problem hiding this comment.
Please rename the inner function check to check_identical for readability.
| sarr = sarr.transpose(inplace=False) | ||
| check(sarr, ndarrT) | ||
|
|
||
| ndarrT = ndarr.transpose(0, 2, 1, 3) |
There was a problem hiding this comment.
Wow, numpy is so .... confusing. I grow more fond of SimpleArray.
|
|
||
| sarr = modmesh.SimpleArrayFloat64(array=ndarr) | ||
| sarr.transpose() | ||
| check(sarr, ndarrT) |
There was a problem hiding this comment.
Could you please also test sarr.transpose().copy()?
There was a problem hiding this comment.
The default paremeter bool inplace is true, and it would return nothing.
There was a problem hiding this comment.
This will be an issue. In that case, there will not be an equivalent of ndarr.transpose.copy().
Let's give it more discussions and revise in a new PR.
ThreeMonth03
left a comment
ThreeMonth03
left a comment
There was a problem hiding this comment.
@yungyuc You could review the pull request when you are available.
| if (axis[it] >= m_shape.size() || axis[it] < 0) | ||
| { | ||
| throw std::runtime_error("SimpleArray: axis out of range"); | ||
| } | ||
| if (new_shape[it] != -1) | ||
| { | ||
| throw std::runtime_error("SimpleArray: axis already set"); | ||
| } |
There was a problem hiding this comment.
Ensure the small_vector axis; is the permutation of the sequence from 0 to n - 1.
| { | ||
| self_copy.transpose(make_shape(axis)); | ||
| } | ||
| return py::cast(std::move(self_copy)); |
There was a problem hiding this comment.
I don't know why the parameter of py::cast() should be rvalue.
| { return self.reshape(make_shape(shape)); }) | ||
| .def( | ||
| "transpose", | ||
| [](wrapped_type & self, py::object axis, bool inplace) -> py::object |
There was a problem hiding this comment.
In order to deal with the in-place function, the return type of transpose() is py::object.
| { return self.reshape(make_shape(shape)); }) | ||
| .def( | ||
| "transpose", | ||
| [](wrapped_type & self, py::object const & axis, bool const & inplace) -> py::object |
There was a problem hiding this comment.
In order to return None when inplace = True, the return type could be py::object.
|
|
||
| sarr = modmesh.SimpleArrayFloat64(array=ndarr) | ||
| sarr.transpose() | ||
| check(sarr, ndarrT) |
There was a problem hiding this comment.
The default paremeter bool inplace is true, and it would return nothing.
| noarr = sarr.transpose() | ||
| check_identical(sarr, ndarrT) | ||
| self.assertEqual(noarr, None) |
There was a problem hiding this comment.
The return type of in-place transpose is None.
There was a problem hiding this comment.
Oh, right. It's OK to leave it behaving like this right now. But I think we will later need to consider how to align the returning behaviors for various array functions that @Tommy-Hsu is planning in https://docs.google.com/document/d/1r6ESbTZPtzmYpnx-sepgp7kXWwwM48XoeVny0dqrg8s .
Also cc @KHLee529 . We will have a lot of fun code to write.
| noarr = sarr.transpose() | ||
| check_identical(sarr, ndarrT) | ||
| self.assertEqual(noarr, None) |
There was a problem hiding this comment.
Oh, right. It's OK to leave it behaving like this right now. But I think we will later need to consider how to align the returning behaviors for various array functions that @Tommy-Hsu is planning in https://docs.google.com/document/d/1r6ESbTZPtzmYpnx-sepgp7kXWwwM48XoeVny0dqrg8s .
Also cc @KHLee529 . We will have a lot of fun code to write.
|
|
||
| sarr = modmesh.SimpleArrayFloat64(array=ndarr) | ||
| sarr.transpose() | ||
| check(sarr, ndarrT) |
There was a problem hiding this comment.
This will be an issue. In that case, there will not be an equivalent of ndarr.transpose.copy().
Let's give it more discussions and revise in a new PR.
|
See the follow-up issue #511 |
yungyuc
changed the title
2 participants
According to issue #506 , I implement the
void SimpleArray::reverse()and wrap it to the python.