refactor: privatize StridedLayout as _StridedLayout#1373
Conversation
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test d61ed6664a574ddd0baa4c3a6ff1e6b614c6a293 |
This comment has been minimized.
This comment has been minimized.
leofang
added
P0
| @@ -152,6 +158,48 @@ cdef class StridedMemoryView: | |||
| return cls.from_dlpack(obj, stream_ptr) | |||
| return cls.from_cuda_array_interface(obj, stream_ptr) | |||
|
|
|||
| @classmethod | |||
| def from_buffer( | |||
| cls, buffer : Buffer, layout : _StridedLayout, | |||
There was a problem hiding this comment.
Happy to defer it to a follow up but having this constructor require a private class isn't ideal. Should we make this private as well?
There was a problem hiding this comment.
I will address these comments in the from_buffer PR follow-up.
| @@ -169,71 +217,153 @@ cdef class StridedMemoryView: | |||
| dlm_tensor = <DLManagedTensor*>data | |||
| dlm_tensor.deleter(dlm_tensor) | |||
|
|
|||
| def view( | |||
| raise NotImplementedError("Sorry, not supported: copy_to") | ||
|
|
||
| @property | ||
| def layout(self) -> _StridedLayout: |
There was a problem hiding this comment.
Should we make this private for now?
There was a problem hiding this comment.
Will do in a follow-up.
d61ed66 to
855d5eb
Compare
|
/ok to test |
|
/ok to test |
|
Let me admin-merge this PR. All other CI pipelines are green but one, which seems to be causing the main branch to fail as well. I've raised a thread internally. |
|
3 participants
Revert the revert of StridedLayout and privatize as
_StridedLayoutSummary
This PR reverts the removal of
StridedLayout(reverting #1370) andimmediately privatizes it to
_StridedLayoutto unblock ongoing work whileavoiding premature API stabilization.
Motivation
The original
StridedLayoutimplementation was reverted in #1370, but thisblocked work that depends on the internal layout infrastructure. Rather than
keeping the code removed, we've chosen to:
StridedLayoutfunctionalityStridedLayoutto_StridedLayout(making it internal-only)This approach allows internal and experimental code to continue using the
layout functionality while clearly signaling that the API is not yet stable.
Changes
StridedLayoutStridedLayoutto_StridedLayoutthroughout the codebase_StridedLayoutfrom public API documentationFollow-up Work
An immediate follow-up PR will refactor
StridedMemoryView.from_buffertoremove the
_StridedLayoutparameter and replace it with explicitstrideandshapearguments. This will further reduce the API surface of the internallayout implementation.