Mojo-Numerics-and-Algorithms-group / NuMojo

NuMojo is a library for numerical computing in Mojo 🔥 similar to numpy in Python.
Apache License 2.0
112 stars 15 forks source link

[change] Rename the data buffer from `data` to `_buffer` #136

Closed forFudan closed 3 weeks ago

forFudan commented 3 weeks ago

The data buffer of NDArray is an unsafepointer. We should not expose it to the users. So it is renamed to _buffer with an underscore.

MadAlex1997 commented 3 weeks ago

The data buffer of NDArray is an unsafepointer. We should not expose it to the users. So it is renamed to _buffer with an underscore.

All this would really do is hide the UnsafePointer declaration in the documentation of NDArray, mojo does not have a public/private interface system to stop users from touching things.

I am not opposed to a name change since data is kind of vague, perhaps pointer or ptr. I feel that it is valid for an advanced user to want to take ownership of the ptr from NDArray, although I struggle to think of a good reason to do that.

forFudan commented 3 weeks ago

All this would really do is hide the UnsafePointer declaration in the documentation of NDArray, mojo does not have a public/private interface system to stop users from touching things.

Yeah, but we have the Pythonic convention that things with single underscore means "private" and ordinary users should not touch it unless knowing the consequence :D

With this name, users will understand that they need to be cautious and do not accidentallyy change the data buffer.

Actually it is also used for the buffer of the String type in the standard library.

forFudan commented 3 weeks ago

For String type:

struct String(
...
):
    """Represents a mutable string."""

    # Fields
    alias _buffer_type = List[UInt8, hint_trivial_type=True]
    var _buffer: Self._buffer_type
shivasankarka commented 3 weeks ago
forFudan commented 3 weeks ago

We already have some methods that expose the UnsafePointer which I think should be allowed for advanced users, so something simple as _ptr seems good.

For sake of simplicity, it can be named as _buf, but _ptr is not a proper name.

On the one hand, ptr stands for the type of the underlying data, but not the purpose (nature) of the data. There can be multiple unsafe pointers in a structure. Using ptr is ambiguous.

On the other hand, for C users, a structure pointer points to the memory address where the structure is stored, not the buffer of the structure. One still needs to do ptr->buffer to get to the underlying buffer. Using ptr for the buffer would be misleading.

That is why I think _buf would be better.

shivasankarka commented 3 weeks ago

@forFudan Yep, _buf seems fine to me. Please update it so that I canl merge it.