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

remove enum defaults, and some other cleanup #7248

Closed
4 tasks
cosmicexplorer opened this issue Feb 15, 2019 · 0 comments · Fixed by #7269
Closed
4 tasks

remove enum defaults, and some other cleanup #7248

cosmicexplorer opened this issue Feb 15, 2019 · 0 comments · Fixed by #7269

Comments

@cosmicexplorer
Copy link
Contributor

@illicitonion left some very useful comments on #7226, one specifically asking why enums have defaults: #7226 (review). This led me to immediately realize that all of the complexity of the argument manipulation with *args, **kwargs in the .create() classmethod mentioned in #7226 (comment) was due to trying to support default values "hygienically", and the only thing that really needed a default value was Platform:

class Platform(enum('normalized_os_name', all_normalized_os_names())):
default_value = get_normalized_os_name()
which really should only expose the current os name as the default value. The default value part was also maybe useful for registering enums as options directly, but we've now laid out how we want to do that in #7233. This ticket covers:

cosmicexplorer added a commit that referenced this issue Mar 2, 2019
### Problem

Resolves #7232, resolves #7248, and addresses #7249 (comment).

### Solution

- Remove enum defaults and the `.create()` method with complicated argument handling.
- Allow enums to be `==` compared as long as they're the same type.
- Allow accessing enum instances with class attributes.

### Result

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

Successfully merging a pull request may close this issue.

1 participant