DisnakeDev
refactor(enums)!: performance and functional improvements
Summary
This PR reworks disnake's custom enums. The existing enums come with a couple issues. First of all, the API does not match that of Python's built-in Enum class. Secondly, existing enums cannot be typed (e.g. class MyEnum(int, Enum)). This makes them both unwieldy and slower than necessary.
This PR makes the following changes:
- Enums now adhere to the vanilla Enum API (need to double-check some field names),
- Enums now must be typed,
- ~~
TODO:all explicit typecasts on enums and all explicitmember.valueaccess have been removed,~~ This will be deferred to a later PR. - implement unit tests for the new enums.
Explicitly typed enums come with the advantage that their members can be directly used in place for anything that accepts its type. For example, an int-enum member could be directly added to an integer:
>>> class Integer(int, Enum):
... ONE = 1
... TWO = 2
>>> # old enums:
>>> Integer.ONE.value + Integer.TWO.value + 3
6
>>> # new enums:
>>> Integer.ONE + Integer.TWO + 3
6
This comes with the advantage of not having to explicitly access value, which is both easier on the developer and faster. This also means that isinstance checks with an enum member and its type will pass, etc. Over the whole library, this means a lot of now unnecessary member access and function calls can be removed. Since enums are used extensively under the hood, this should lead to a small speed increase. A screenshot of tentative benchmarks of the projected performance increase can be seen here:
Lastly, because the API now more closely matches that of the default Enum implementation, these enums should be usable as a drop-in replacement for enums in a wide range of use cases, especially where the speed increase is necessary.
Checklist
- [x] If code changes were made, then they have been tested
- [ ] I have updated the documentation to reflect the changes
- [x] I have formatted the code properly by running
task lint - [x] I have type-checked the code by running
task pyright
- [x] This PR fixes an issue
- [ ] This PR adds something new (e.g. new method or parameters)
- [x] This PR is a breaking change (e.g. methods or parameters removed/renamed)
- [ ] This PR is not a code change (e.g. documentation, README, ...)
One thing that needs to be made sure, is that commands.option_enum() isn't broken, and that stdlib enums are supported for options but also allow internal enums. This may already be done, but I'm writing it down anyways.
I think this can be reviewed, and as you said, we can change accessors of enums to not have the value attribute in a later pull request.
@Chromosomologist would you please resolve conflicts on this pr and get it ready for a review?
@Chromosomologist would you please resolve conflicts on this pr?
After some amount of struggle-- done ^^
Still need to get around to implementing a check to prevent messy inheritance, and I'm still undecided about what to do with duplicates. The other feedback should all be incorporated now.
As a heads-up, further enum inheritance is now supported such that e.g. methods can be carried over to all subclasses:
>>> class A(Enum):
... def hi(self):
... print("hi")
>>> class X(int, A):
... a = 1
>>> X.a.hi()
hi
However, to define members, we do still require the enum to have its type specified.
I've deployed the latest version of this branch (as of this comment, https://github.com/DisnakeDev/disnake/pull/556/commits/5929b4668e049919ca6147643c12e5d4ec0fb6db) on Monty.
This has a bit of an issue with proxy invalid values, for example a message type of 25 would end up as a simple integer instead of a enum value.
All changes to the underlying enum logic are done, assuming we don't run into any bugs over the testing period. As far as this PR goes, everything should be final.
As for the rest, enum subclasses' docs will need to be reintroduced as part of a separate PR, and we can then start looking into performance optimizations across the lib, too.