click
click copied to clipboard
Add `EnumChoice` parameter type
Added a new ability to be able to create a choice parameter type from enum class.
How does it differ from Choice
? Enums are the way of python to allow only a finite number of values to be accepted in a given scenario. It seems logical to me that click should offer a Choice
-like parameter type that gets its values from an enum and returns an enum.
Now, using click.EnumChoice
, one can create a Choice
-like parameter that returns an enum instead of a string.
Moreover, once you add a new value to your enum class, it will automatically update as a valid choice for the paramater type.
- [X] Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
- [ ] Add or update relevant docs, in the docs folder and in code.
- [ ] Add an entry in
CHANGES.rst
summarizing the change and linking to the issue. - [ ] Add
.. versionchanged::
entries in any relevant code docs. - [X] Run
pre-commit
hooks and fix any issues. - [X] Run
pytest
andtox
, no tests failed.
Hey @davidism .
I would love to hear your feedback on this PR.
Does this PR need any changes?
Is it click
8.2.0 material? 9.0.0?
I would love to know so I could work on it if any changes are necessary.
I think this feature could really help the users of click
.
The PR itself seems fine, but I'm not sure I want to merge it yet. You'll know when I make a decision because I'll update it one way or another.
Related to #605.
Why wouldn't you want to merge that? (Serious question)
What's in favor:
- Enums have been in the stdlib for a very long time now and lead to cleaner and more robust code for exactly these kind of "choice problems".
- This PR looks quite clean. It adds little new code and has many tests.
- There seems to be a high demand for this feature since nearly 6 years. People have been updating the example in the Issue if it stopped working after a click update.
Looking forward to hearing your objections, @davidism :-)
@saroad2 If David would consider merging this PR, you should add tests using EnumChoice
with a default value (because you cannot use MyEnum.option
have to use MyEnum.option.name
. And I assume you will also add documentation than? :-)
Hey @sscherfke , once David will approve that he intend on merging this PR (in the near or far future) I will add the relevant documentation and add any test that might be relevant.
Hey @davidism, have you been able to make a decision? 😃 I think @saroad2 would love to move forward with the PR and get this available for all Click users.
This PR still waits for @davidism approval.
I know there are issues about testing and API in this PR, but I can't work on it until David do an initial review.
Would love to see it getting merged.
@davidism i'd love to see this land, after all i just shipped a broken release of a cli due to a mistake with removing a choice and missing a purely stringly usage on a default all the tests would pass in correctly
had i been able to use enum i wouldn't have missed that
Hey @mjpieters, thanks for the review.
Are you from the Pallets team? Until someone from them take the time and address this PR, I'm not going to spend time fixing issues here. This PR is sitting here for over a year. I thought it would get merged by now.
i'll try to bring it up again on the discord, i'd love to see it land but i dont call the shot
Click or Jinja is my next focus, as soon as the new Flask and Werkzeug releases settle down. I had been focused almost entirely on Flask and Werkzeug for the last year.
Hey @mjpieters, thanks for the review.
Are you from the Pallets team? Until someone from them take the time and address this PR, I'm not going to spend time fixing issues here. This PR is sitting here for over a year. I thought it would get merged by now.
I'm not a member of the pallets team, no, sorry.
I have been using this implementation in production code, and found aliases to be a great way to improve a CLI where the spelling of some of the choices could cause confusion. Aliases solved those issues very, very nicely, without cluttering up the UI further.
So, I'd really love to see this become part of core click package, but with some form of aliases support included.
Click or Jinja is my next focus, as soon as the new Flask and Werkzeug releases settle down. I had been focused almost entirely on Flask and Werkzeug for the last year.
Thanks @davidism , I appriciate you. I hope to see it merged soon.
Click is used by a lot of users, and personally I find myself use it in most of my projects. Maybe it's time to consider adding more maintainers to this project for more routinely checks of this project. If you open up a call, I would happily suggest myself. I'm sure many others would too.
As for this perticular PR, once you review it, I would gladly start fixing issues here.
I'm definitely open to adding more long-term contributors, especially around reviewing existing PRs and triaging issues. Ping me in https://discord.gg/pallets #click
channel about getting set up.
@saroad2 (+@mjpieters)
It might also be nice to allow constructing enums from member values and not keys, i.e. by using Enum(x)
as opposed to Enum[x]
. Note that Enum(x)
would also respond to the built-in _missing_()
functionality allowing you to concoct whatever you want in terms of aliases etc whereas Enum[x]
would not.
Using __getitem__
on the enum type itself is pretty restrictive since in order to override it you need to mess with metaclasses whereas __init__
allows for the _missing_()
workaround.
E.g. in cases like
class MyEnum(Enum):
SNAKE_CASE_NAME = 'first'
TOO_MANY_UNDERSCORES = 'second'
@classmethod
def _missing_(cls, obj):
if obj == '1':
return cls('first')
elif obj == '2':
return cls('second')
return None
you would probably want it to respond to 1
, 2
, first
and second
rather than SNAKE_CASE_NAME
and TOO_MANY_UNDERSCORES
in the CLI.
This might also partially solve the case-sensitivity question (in fact, that's the example for _missing_()
in the official docs).
Hey everyone!
I'm glad to see so many changes to Click since I last logged in to this project! I'd happily work on this PR and prepare it for the merge.
But first, I still need @davidism, as the primary maintainer of this project, to review it beforehand since this is a new feature.
Once that happens, I'll work on addressing everyone's comments.
Meanwhile, I merged the main branch into the PR to make it up-to-date.