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

Add EnumChoice parameter type #2210

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/click/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
from .types import BOOL as BOOL
from .types import Choice as Choice
from .types import DateTime as DateTime
from .types import EnumChoice as EnumChoice
from .types import File as File
from .types import FLOAT as FLOAT
from .types import FloatRange as FloatRange
Expand Down
18 changes: 18 additions & 0 deletions src/click/types.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import collections.abc as cabc
import enum
import os
import stat
import sys
Expand Down Expand Up @@ -336,6 +337,23 @@ def shell_complete(
return [CompletionItem(c) for c in matched]


class EnumChoice(Choice):
def __init__(self, enum_type: type[enum.Enum], case_sensitive: bool = True):
super().__init__(
choices=[element.name for element in enum_type],
Copy link
Contributor

@mjpieters mjpieters May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider including aliases in this; either as an explicit option (include_aliases: bool = False) or by default. Loop over the enum_type.__members__ mapping to get all names including aliases:

Suggested change
choices=[element.name for element in enum_type],
choices=[name for name in enum_type.__members__],

Or, and this may be even better, map aliases to canonical names in convert(), before passing on the value to super().convert(), so that the choices listed in help documentation don't include the aliases.

In that case, store the aliases here in __init__ and reference those in convert():

        # ...
        self.enum_type = enum_type
        self.aliases = {
            alias: enum_type[alias].name
            for alias in set(enum_type.__members__).difference(self.choices)
        }

    def convert(
        self, value: t.Any, param: t.Optional["Parameter"], ctx: t.Optional["Context"]
    ) -> t.Any:
        value = self.aliases.get(value, value)
        value = super().convert(value=value, param=param, ctx=ctx)
        # ...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't that break things like the case_sensitive flag?
Like, if you have something like:

import enum

class MyEnum(enum.Enum):
    Foo = 0
    Bar = 1
    Baz = 1  # Alias for `Bar`

# Inside `EnumChoice`:
choices = ["Foo", "Bar"]
aliases = {"Baz": "Bar"}

# And in the `EnumChoice.convert` method:
value = "baz"  # User input
value = aliases.get(value, value)  # "baz" not in `aliases`
# `value` is still "Baz"
# When passed to `super().convert(...)`, it won't know what to do with it

I guess this would either require reimplementing the case_sensitive handling inside EnumChoice.convert, or actually having all possible values, including aliases, in EnumChoice.choices.

In my opinion, reimplementing the flag would be sketchy, so I'd probably suggest having all aliases in the choices.

One thing that might be possible, but I have no idea if it actually is, would be to then modify how the argument documentation is generated, to either omit the aliases or to document them together with the primary choice.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not having aliases in EnumChoice.choices would also break shell completion for the aliases. This may or may not be undesirable.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing, on your suggested change you use:

choices=[name for name in enum_type.__members__],

But the following would also work:

choices=list(enum_type.__members__)

There's also enum_type.__members__.keys(), but that's not a sequence, it's just iterable.

Copy link
Contributor

@mjpieters mjpieters May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh, yes, list(enum_type.__members__) is the obvious choice. I also don't use the case-sensitive option so I indeed missed that.

In that case, case-fold the aliases in the mapping:

        # ...
        self.enum_type = enum_type
        self.aliases = {
            alias if case_sensitive else alias.casefold(): enum_type[alias].name
            for alias in set(enum_type.__members__).difference(self.choices)
        }
        
    def convert(
        self, value: t.Any, param: t.Optional["Parameter"], ctx: t.Optional["Context"]
    ) -> t.Any:
        value = self.aliases.get(value if self.case_sensitive else value.casefold(), value)
        value = super().convert(value=value, param=param, ctx=ctx)
        # ...

This does start to get somewhat repetitive with the code in Choice, and I did ignore the optional ctxt.token_normalize_func() function. Perhaps the normalisation (case folding, ctx.token_normalize_func() support) could be factored out into a supporting function that can then be reused here. Something that generates a transformation function based on the case_sensitive flag and ctx.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not having all values in choices will also break shell_complete I believe.

Copy link
Contributor

@mjpieters mjpieters May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not having all values in choices will also break shell_complete I believe.

No, it won't break completion. You won't be able to use the aliases as completion words but that's not necessarily the point of them, but nothing breaks when you try to complete against a partial alias. You just don't get a completed word at that point.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Should there be the ability to use the enum values as choices, enabled through a parameter?
  • Should there be the ability to use the enum member itself as the default, which I believe would require adding the enum members themselves to choices, not just their string name? e.g. default=SomeEnum.member

case_sensitive=case_sensitive,
)
self.enum_type = enum_type

def convert(
self, value: t.Any, param: Parameter | None, ctx: Context | None
) -> t.Any:
value = super().convert(value=value, param=param, ctx=ctx)
if value is None:
return None
return self.enum_type[value]


class DateTime(ParamType):
"""The DateTime type converts date strings into `datetime` objects.

Expand Down
52 changes: 52 additions & 0 deletions tests/test_basic.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import enum
import os
from itertools import chain

Expand All @@ -6,6 +7,15 @@
import click


class MyEnum(enum.Enum):
"""Dummy enum for unit tests."""

ONE = "one"
TWO = "two"
THREE = "three"
ONE_ALIAS = ONE


def test_basic_functionality(runner):
@click.command()
def cli():
Expand Down Expand Up @@ -403,6 +413,48 @@ def cli(method):
assert "{foo|bar|baz}" in result.output


def test_enum_choice_option(runner):
@click.command()
@click.option("--number", type=click.EnumChoice(MyEnum))
def cli(number):
click.echo(number)

result = runner.invoke(cli, ["--number=ONE"])
assert not result.exception
assert result.output == "MyEnum.ONE\n"

result = runner.invoke(cli, ["--number=meh"])
assert result.exit_code == 2
assert (
"Invalid value for '--number': 'meh' is not one of 'ONE', 'TWO', 'THREE'."
in result.output
)

result = runner.invoke(cli, ["--help"])
assert "--number [ONE|TWO|THREE]" in result.output


def test_enum_choice_argument(runner):
@click.command()
@click.argument("number", type=click.EnumChoice(MyEnum))
def cli(number):
click.echo(number)

result = runner.invoke(cli, ["ONE"])
assert not result.exception
assert result.output == "MyEnum.ONE\n"

result = runner.invoke(cli, ["meh"])
assert result.exit_code == 2
assert (
"Invalid value for '{ONE|TWO|THREE}': 'meh' is not one of 'ONE', "
"'TWO', 'THREE'." in result.output
)

result = runner.invoke(cli, ["--help"])
assert "{ONE|TWO|THREE}" in result.output


def test_datetime_option_default(runner):
@click.command()
@click.option("--start_date", type=click.DateTime())
Expand Down
22 changes: 22 additions & 0 deletions tests/test_info_dict.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import enum

import pytest

import click.types

# Common (obj, expect) pairs used to construct multiple tests.

STRING_PARAM_TYPE = (click.STRING, {"param_type": "String", "name": "text"})
INT_PARAM_TYPE = (click.INT, {"param_type": "Int", "name": "integer"})
BOOL_PARAM_TYPE = (click.BOOL, {"param_type": "Bool", "name": "boolean"})
Expand Down Expand Up @@ -91,6 +94,15 @@
)


class MyEnum(enum.Enum):
"""Dummy enum for unit tests."""

ONE = "one"
TWO = "two"
THREE = "three"
ONE_ALIAS = ONE


@pytest.mark.parametrize(
("obj", "expect"),
[
Expand All @@ -115,6 +127,16 @@
},
id="Choice ParamType",
),
pytest.param(
click.EnumChoice(MyEnum),
{
"param_type": "EnumChoice",
"name": "choice",
"choices": ["ONE", "TWO", "THREE"],
"case_sensitive": True,
},
id="EnumChoice ParamType",
),
pytest.param(
click.DateTime(["%Y-%m-%d"]),
{"param_type": "DateTime", "name": "datetime", "formats": ["%Y-%m-%d"]},
Expand Down
21 changes: 21 additions & 0 deletions tests/test_normalization.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
import enum

import click

CONTEXT_SETTINGS = dict(token_normalize_func=lambda x: x.lower())


class MyEnum(enum.Enum):
"""Dummy enum for unit tests."""

ONE = "one"
TWO = "two"
THREE = "three"
ONE_ALIAS = ONE


def test_option_normalization(runner):
@click.command(context_settings=CONTEXT_SETTINGS)
@click.option("--foo")
Expand All @@ -25,6 +36,16 @@ def cli(choice):
assert result.output == "Foo\n"


def test_enum_choice_normalization(runner):
@click.command(context_settings=CONTEXT_SETTINGS)
@click.option("--choice", type=click.EnumChoice(MyEnum))
def cli(choice):
click.echo(choice)

result = runner.invoke(cli, ["--CHOICE", "ONE"])
assert result.output == "MyEnum.ONE\n"


def test_command_normalization(runner):
@click.group(context_settings=CONTEXT_SETTINGS)
def cli():
Expand Down
71 changes: 71 additions & 0 deletions tests/test_options.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import enum
import os
import re

Expand All @@ -7,6 +8,15 @@
from click import Option


class MyEnum(enum.Enum):
"""Dummy enum for unit tests."""

ONE = "one"
TWO = "two"
THREE = "three"
ONE_ALIAS = ONE


def test_prefixes(runner):
@click.command()
@click.option("++foo", is_flag=True, help="das foo")
Expand Down Expand Up @@ -571,6 +581,67 @@ def cmd(foo):
assert result.output == "Apple\n"


def test_missing_enum_choice(runner):
@click.command()
@click.option("--foo", type=click.EnumChoice(MyEnum), required=True)
def cmd(foo):
click.echo(foo)

result = runner.invoke(cmd)
assert result.exit_code == 2
error, separator, choices = result.output.partition("Choose from")
assert "Error: Missing option '--foo'. " in error
assert "Choose from" in separator
assert "ONE" in choices
assert "TWO" in choices
assert "THREE" in choices
assert "ONE_ALIAS" not in choices


def test_case_insensitive_enum_choice(runner):
@click.command()
@click.option("--foo", type=click.EnumChoice(MyEnum, case_sensitive=False))
def cmd(foo):
click.echo(foo)

result = runner.invoke(cmd, ["--foo", "one"])
assert result.exit_code == 0
assert result.output == "MyEnum.ONE\n"

result = runner.invoke(cmd, ["--foo", "tHREE"])
assert result.exit_code == 0
assert result.output == "MyEnum.THREE\n"

result = runner.invoke(cmd, ["--foo", "Two"])
assert result.exit_code == 0
assert result.output == "MyEnum.TWO\n"

@click.command()
@click.option("--foo", type=click.EnumChoice(MyEnum))
def cmd2(foo):
click.echo(foo)

result = runner.invoke(cmd2, ["--foo", "one"])
assert result.exit_code == 2

result = runner.invoke(cmd2, ["--foo", "tHREE"])
assert result.exit_code == 2

result = runner.invoke(cmd2, ["--foo", "TWO"])
assert result.exit_code == 0


def test_case_insensitive_enum_choice_returned_exactly(runner):
@click.command()
@click.option("--foo", type=click.EnumChoice(MyEnum, case_sensitive=False))
def cmd(foo):
click.echo(foo)

result = runner.invoke(cmd, ["--foo", "ONE"])
assert result.exit_code == 0
assert result.output == "MyEnum.ONE\n"


def test_option_help_preserve_paragraphs(runner):
@click.command()
@click.option(
Expand Down