gh-91484: Allow memoryview cast for F-contiguous#137803
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
StanFromIreland
left a comment
StanFromIreland
left a comment
There was a problem hiding this comment.
This change needs a NEWS entry, you can do it via the web interface (comment above) or with the blurb tool locally. You also have some trailing whitespace.
|
@corona10 Please check my Pull Request |
|
@StanFromIreland thank you! I added NEWS entry. |
corona10
left a comment
corona10
left a comment
There was a problem hiding this comment.
Please check lint status, looks like there is empty space in some line.
|
Ok, I removed the trailing whitespace. |
|
This PR is stale because it has been open for 30 days with no activity. |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Looks fine. I added some minor suggestions, and please update Doc/library/stdtypes.rst which says "Supported casts are 1D -> C-:term:contiguous and C-contiguous -> 1D." Mention that an F-contiguous can also be cast to 1D. Add versionchanged:: next and a What's New entry.
I just submitted a complementary PR #153663. They can be merged in arbitrary order, but the latter will need to resolve conflict in What's New.
| PyErr_SetString(PyExc_TypeError, | ||
| "memoryview: casts are restricted to C-contiguous views"); | ||
| return NULL; | ||
| if(shape || self->view.ndim == 1 || !MV_F_CONTIGUOUS(self->flags)) { |
There was a problem hiding this comment.
Missed space, and self->view.ndim == 1 looks redundant -- for 1D view C- and F-contiguity are equivalent.
| if(shape || self->view.ndim == 1 || !MV_F_CONTIGUOUS(self->flags)) { | |
| if (shape || !MV_F_CONTIGUOUS(self->flags)) { |
| return NULL; | ||
| if(shape || self->view.ndim == 1 || !MV_F_CONTIGUOUS(self->flags)) { | ||
| PyErr_SetString(PyExc_TypeError, | ||
| "memoryview: casts are restricted to C-contiguous views"); |
There was a problem hiding this comment.
It is now slightly misleading, since F-contiguous -> 1D is allowed.
Maybe just "contiguous views"?
| "memoryview: casts are restricted to C-contiguous views"); | |
| "memoryview: casts are restricted to contiguous views"); |
| @@ -0,0 +1 @@ | |||
| memoryview.cast() now allows casting from N-D to 1-D for F-contiguous. | |||
There was a problem hiding this comment.
| memoryview.cast() now allows casting from N-D to 1-D for F-contiguous. | |
| :meth:`memoryview.cast` now allows casting from N-D to 1-D for F-contiguous. |
4 participants
Allow N-D to 1-D memoryview cast for F-contiguous
memoryviews incast#91484