Skip to content

#[pyclass] on enums MVP #2013

Closed

Description

@davidhewitt

After #2002 it's legal to use #[pyclass] on enums that have only unit variants.

However, there's a few pieces which need finishing (at a minimum) before releasing this change in 0.16:

Activity

  1. davidhewitt commented on Nov 21, 2021

    @davidhewitt
    MemberAuthor

    (Also, if anyone has strong opinion that this to be a separate #[pyenum] macro instead of part of #[pyclass], it would be better to split it out before 0.16. Personally I quite like having just the one #[pyclass] macro.)

  2. jovenlin0527 commented on Nov 21, 2021

    @jovenlin0527
    Contributor

    After #2002 it's legal to use #[pyclass] on enums that have only unit variants.

    However, there's a few pieces which need finishing (at a minimum) before releasing this change in 0.16:

    • The generated class most likely wants a __richcmp__ implementation so that comparisons work as expected
    • __repr__ implementation

    How about we introduce a new trait in src/class/impl_.rs that allows us to add default implementations for magic methods? This may be useful for other situations.

    • The generated class may want a metaclass to prevent subclassing

    Related to #906.

    It's not clear what's the expected behavior. Python's IntEnum subclasses from int, but that's not currently supported in PyO3, and it makes the enum immutable.

    • CHANGELOG entry
  3. davidhewitt commented on Nov 21, 2021

    @davidhewitt
    MemberAuthor

    How about we introduce a new trait in src/class/impl_.rs that allows us to add default implementations for magic methods? This may be useful for other situations.

    Agreed for sure. See also #1375 / #1376 which spoke about making "dataclass"-style #[pyclass] with hash, eq etc.

    Related to #906.

    Yes I think we can live without this for now as it's an ergonomics improvement in my eyes rather than core functionality.

    It's not clear what's the expected behavior.

    For C-style enums in #834, I would suggest that if comparisons against integers work as expected then that's probably a great starting point. We will probably want to get feedback from users as to how they want to interact with these things, and shouldn't be afraid of making changes after 0.16 once we do get some feedback.

  4. davidhewitt commented on Nov 21, 2021

    @davidhewitt
    MemberAuthor

    We could also solve the subclassing-from-int issue if you think that's necessary. I don't think we're that far out from achieving it.

  5. jovenlin0527 commented on Nov 21, 2021

    @jovenlin0527
    Contributor

    For C-style enums in #834, I would suggest that if comparisons against integers work as expected then that's probably a great starting point. We will probably want to get feedback from users as to how they want to interact with these things, and shouldn't be afraid of making changes after 0.16 once we do get some feedback.

    I don't have a strong opinion on how to implement that. I suggested subclassing int because that's what IntEnum does and I assumed that's what the user would expect, but I never used IntEnum so I'm not really sure. I think it's fine to implement that in a simple way (i.e. __int__ and __eq__) and see how users will think.

  6. added this to the 0.16 milestone on Nov 22, 2021
  7. jovenlin0527 commented on Nov 24, 2021

    @jovenlin0527
    Contributor

    On C-style enums. Rust generates a default discriminant for every variant that doesn't have a discriminant. But also Rust supports any expression (e.g. 2 + 2) as custom discriminants. That means if the user used such expression in their custom discriminants, then we cannot find the default discriminant for other variants without evaluating those expressions. I think we either ban any expressions that are not literals, or we don't support default discriminant. Or both.

    By the way, do we treat all enums without data as C-style enums, even if there's no custom discriminant in the enum?

  8. birkenfeld commented on Nov 24, 2021

    @birkenfeld
    Member

    Why do you need to calculate the discriminants? Can't you just always use Enum::Var as uXX in the generated code?

  9. jovenlin0527 commented on Nov 24, 2021

    @jovenlin0527
    Contributor

    I didn't know you can cast them to uXX 😅. Yeah let's cast to usize.

  10. linked a pull request that will close this issue#[pyclass] for fieldless enums #2034on Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions