Skip to content
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

Convert arrays to tuple instead of list #4837

Closed
MartinJepsen opened this issue Jan 5, 2025 · 5 comments
Closed

Convert arrays to tuple instead of list #4837

MartinJepsen opened this issue Jan 5, 2025 · 5 comments

Comments

@MartinJepsen
Copy link

MartinJepsen commented Jan 5, 2025

With this simple #[pyclass]:

#[pyclass(get_all)]
struct Example {
    inner: [u32; 3]
}

the Python equivalent becomes

class Example:
    inner: list[int]

I would argue that since arrays in Rust have a fixed length, it would make more sense to convert this to a tuple, such that be Python class becomes

class Example:
    inner: tuple[int, int, int]

I don't know if there are any technical limitations that go against this suggestion, but I think it would make sense to convert fixed-length sequence types to tuple.

Other thoughts/questions

  • Is there a way to manually specify how a Rust library type to a pyo3 type? Perhaps, something like this pseudo-code example is possible:

    #[pyclass(get_all)]
    struct Example {
        #[pyo3(type="tuple[int, int, int]")]   // Maybe like this
        #[pyo3(type=pyo3::types::PyTuple)]     // or like this
        inner: [u32; 3]

    I couldn't find anything in the docs about how to do this in the docs, but if it's there please let me know! It would also help in cases where you dont want a [u8; N] to come out as bytes.

  • You could argue that the type of inner should be (u32, u32, u32), as according to the conversion table, this will map to a Python tuple. It will cause some longer type annotations of course (in my code base I have 3x3 matrices, typed as [[f32; 3]; 3], the alternative would be

    (
        (f32, f32, f32),
        (f32, f32, f32),
        (f32, f32, f32),
    )

    but I think this is OK. Don't know if there's a performance penalty to this.

@davidhewitt
Copy link
Member

davidhewitt commented Jan 14, 2025

While your points are very reasonable, I think this is unlikely to be a change that we'd make. Aside from the churn that this would cause for existing code, there's risk of possible inconsistencies if [T; N] converted to a tuple.

  • What should &[T; N] convert to? What about &[T]? What about &Vec<T>? Both &[T; N] and &Vec<T> will deref to &[T] and it seems like an easy footgun if these three types might convert to different things depending on whether you dereference them?
  • What about &[u8; 3]? Should this be a 3-element integer tuple? &[u8] is already a special case that produced Python bytes. It seems reasonable from the above bullet that &[u8; N] should also produce bytes.

The above bullets both suggest that &[T; N] should convert like &[T]. Similarly to the above, why should [T; N] convert any differently to &[T; N]?


For your alternative #[pyo3(type=pyo3::types::PyTuple)] suggestion, maybe #[pyo3(into_py_with)] from #4850 could theoretically help? You can also already write a fn get_inner() in #[pymethods] manually to do the conversion you want.

@Icxolu
Copy link
Contributor

Icxolu commented Jan 14, 2025

I agree here. I also tend to think about arrays (from a Rust perspective) as a homogeneous collections which puts it in the same bucket as a slice or vector. I think it would very surprising to handle arrays differently from these collections since there are so closely related in Rust. It's true that in Python lists don't have to be homogeneous, but they often are as well. From tuples on the other hand I think it is more likely to expect it may be heterogeneous.

I think for the specific example #[pyo3(into_py_with)] could help and is a good way to support such modifications IMO. In it's current implementation this would still need a wrapper type, because it's only implemented for #[derive(IntoPyObject)] (and not pyclasses) for now, but it should be possible to support it for pyo3(get/get_all) as well in a followup to #4850.

@MartinJepsen
Copy link
Author

there's risk of possible inconsistencies if [T; N] converted to a tuple.

@davidhewitt thanks for pointing those cases out. It's very interesting to learn some of the nuances that aren't obvious (to me at least) at surface level, so thanks for explaining.

maybe #[pyo3(into_py_with)] from #4850 could theoretically help?

I'll give this a shot, thanks for the tip!

@MartinJepsen
Copy link
Author

It's true that in Python lists don't have to be homogeneous, but they often are as well. From tuples on the other hand I think it is more likely to expect it may be heterogeneous.

@Icxolu Definitely agree, but to clarify, it's not so much the homogenousness that's the motivation for this issue. It's actually the size (number of items). Tuples in Python have a fixed size at runtime and you can tell a type checker what that size is. For example, it makes sense to describe a point in 3D space as a tuple[float, float, float] instead of just a list[float]. But with David's points in mind, I don't think it's worth making PyO3 translate [T; N] to tuple[T, ...]

@Icxolu
Copy link
Contributor

Icxolu commented Jan 16, 2025

Definitely agree, but to clarify, it's not so much the homogenousness that's the motivation for this issue.

Yes, I got that. This was just meant as a different viewing angle on why list also makes sense (and for me personally has a stronger value than fixed vs dynamic size here).

maybe #[pyo3(into_py_with)] from #4850 could theoretically help?

I'll give this a shot, thanks for the tip!

Another option to solve this is to just define your own #[getter] instead of using #[pyo3(get)]. That way you can define whatever output type you like and don't have to wait for into_py_with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants