Consider support for tuple variants in complex enums

[mkovaxx]

Allow the following: https://github.com/PyO3/pyo3/blob/d1b072222a9a166b3b7078b8e1121b4a794d12bb/tests/ui/invalid_pyclass_enum.rs#L24-L28

[davidhewitt]

Are there any design blockers here, or is it just a case of someone with sufficient time taking on the implementation? I would assume that it works very similarly to what we've already done for struct variants.

Thinking about it, I suspect that the only case which we might want to think about specially is the "newtype" form:

#[pyclass]
enum TupleVariants {
    String(String),
    Int(i64),
}

For #[derive(FromPyObject)] there's a special case here to convert these from the inner type rather than as a 1-element tuple. We might want to think about if it makes sense to do anything like this for enums.

[mkovaxx]

Apart from the obvious things like implementing __len__() and __getitem__() on the variant PyClasses (instead of named field accessor methods), there's a detail around the match syntax that seems important for good ergonomics: the __match_args__ attribute.

The match_args attribute is always looked up on the type object named in the pattern. If present, it must be a list or tuple of strings naming the allowed positional arguments.

In deciding what names should be available for matching, the recommended practice is that class patterns should be the mirror of construction; that is, the set of available names and their types should resemble the arguments to init().

Having this in place would allow the patterns in Python match statements to mirror the tuple patterns in Rust match expressions.

For example, given the following Rust enum:

#[pyclass]
enum MyEnum {
    Tuple(i32, f64, String),
}

The following match pattern would be supported in Python:

match variant:
    case MyEnum.Tuple(a, b, c):
        assert isinstance(a, int)
        assert isinstance(b, float)
        assert isinstance(c, str)

Source: https://peps.python.org/pep-0622/#special-attribute-match-args

[davidhewitt]

~~If I follow correctly, the __match_args__ define exactly the names which are available? So in the example you give above, __match_args__ were a, b, c?~~

~~If that's the case, I might suggest we go for _0, _1, _2 etc as the "default" names, as that's probably the closest we can get to rust's .0, .1, .2 numbered field access. The user could then configure these with an attribute:~~

#[pyclass]
enum MyEnum {
    #[pyo3(match_args = (a, b, c))]
    Tuple(i32, f64, String),
}

~~What do you think of that?~~

EDIT: this was incorrect, see below.

[davidhewitt]

... or maybe I'm misunderstanding, and __match_args__ translates positional identifiers to actual names to lookup? So this never needs to be user-customisable, and we always just set __match_args__ to be _0, _1, etc. , and expose the fields as those names too?

[mkovaxx]

If I follow correctly, the __match_args__ define exactly the names which are available? So in the example you give above, __match_args__ were a, b, c?

In the pattern case MyEnum.Tuple(a, b, c):, a, b, c are the names of the bindings that come into existence on a successful match. The user may choose those names arbitrarily in each pattern.

... or maybe I'm misunderstanding, and __match_args__ translates positional identifiers to actual names to lookup? So this never needs to be user-customisable, and we always just set __match_args__ to be _0, _1, etc. , and expose the fields as those names too?

Yep, that's correct!

If that's the case, I might suggest we go for _0, _1, _2 etc as the "default" names, as that's probably the closest we can get to rust's .0, .1, .2 numbered field access. The user could then configure these with an attribute:

#[pyclass]
enum MyEnum {
    #[pyo3(match_args = (a, b, c))]
    Tuple(i32, f64, String),
}

What do you think of that?

I agree that _0, _1 and so on are good names for the generated field accessors. (We would also need such names in the argument list of the generated constructor.) The names of these accessors would then appear in the value of the __match_args__ property.

I tend to err on the side of minimalism so I think it's better not to allow the user to override the "field names" of a tuple variant. If they want to override the names, they should be using a struct variant instead. :)

[davidhewitt]

I tend to err on the side of minimalism so I think it's better not to allow the user to override the "field names" of a tuple variant. If they want to override the names, they should be using a struct variant instead. :)

Yep completely agree, ignore my suggestion of #[pyo3(match_args = (a, b, c))], that was based purely on a brief misunderstanding!

[Stargateur]

I can't do:

#[pyclass]
#[derive(Debug, FromPyObject)]
pub enum Kind {
    Foo {},
    Python { inner: PyObject },
}

error: cannot derive FromPyObject for empty structs and variants

I did that:

#[pyclass]
#[derive(Debug, FromPyObject)]
pub enum FooKind {
    Bar { inner: BarKind },
    Python { inner: PyObject },
}

#[pyclass]
#[derive(Debug, Clone)]
pub enum BarKind {
    FooBar,
}

But that not what I wanted and I don't think I can implement FromPyObject manually without cheating by looking what pyclass is doing.

[davidhewitt]

Why do you need FromPyObject here? To avoid copying the #[pyclass] from Python usually you want to work with Py<Kind>, Bound<Kind> or PyRef<Kind>, which are all wrappers around the Python instance.

[Stargateur]

Well, I'm still learning pyo3, for now I'm just happy when thing work haha, so there wrapper doesn't do conversion, good to know.