Skip to content

Commit

Permalink
Remove type_annotation from OneUseOption
Browse files Browse the repository at this point in the history
This trades one complexity for another:
- the class no longer needs to support deferred/string-ized
  annotations
- it now needs to implement some type deduction logic of its own
  • Loading branch information
sirosen committed Dec 21, 2023
1 parent 7c99fa8 commit 2af5484
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 33 deletions.
3 changes: 0 additions & 3 deletions src/globus_cli/commands/endpoint/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
@one_use_option(
"--personal",
is_flag=True,
type_annotation=bool,
help=(
"Create a Globus Connect Personal endpoint. "
"Mutually exclusive with --server and --shared."
Expand All @@ -43,7 +42,6 @@
@one_use_option(
"--server",
is_flag=True,
type_annotation=bool,
help=(
"Create a Globus Connect Server endpoint. "
"Mutually exclusive with --personal and --shared."
Expand All @@ -57,7 +55,6 @@
"Create a shared endpoint hosted on the given endpoint and path. "
"Mutually exclusive with --personal and --server."
),
type_annotation="tuple[uuid.UUID, str] | None",
)
@mutex_option_group("--shared", "--server", "--personal")
@LoginManager.requires_login("transfer")
Expand Down
1 change: 0 additions & 1 deletion src/globus_cli/commands/endpoint/local_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
"--personal",
is_flag=True,
default=True,
type_annotation=bool,
help="Use local Globus Connect Personal endpoint (default)",
)
def local_id(*, personal: bool) -> None:
Expand Down
32 changes: 12 additions & 20 deletions src/globus_cli/parsing/param_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,6 @@
C = t.TypeVar("C", bound=t.Union[click.BaseCommand, t.Callable])


def _eval_annotation(annotation: str) -> type:
import uuid

# globals for this eval are whatever module names were used in
# OneUseOption annotations
# for now, only 'uuid'
return t.cast(type, eval(annotation, {"uuid": uuid}, {}))


class OneUseOption(click.Option):
"""
Overwrites the type_cast_value function inherited from click.Parameter
Expand All @@ -26,22 +17,26 @@ class OneUseOption(click.Option):
def __init__(
self,
*args: t.Any,
type_annotation: type | str | None = None,
**kwargs: t.Any,
) -> None:
super().__init__(*args, **kwargs)
if type_annotation is None:
raise TypeError("OneUseOption requires a type annotation.")
self._type_annotation = type_annotation

def has_explicit_annotation(self) -> bool:
return True

@property
def type_annotation(self) -> type:
if isinstance(self._type_annotation, str):
self._type_annotation = _eval_annotation(self._type_annotation)
return self._type_annotation
from globus_cli.parsing.param_types import EndpointPlusPath

if self.count:
return bool

if isinstance(self.type, EndpointPlusPath):
return self.type.get_type_annotation(self) | None # type: ignore[return-value] # noqa: E501

raise NotImplementedError(
"OneUseOption requires a type annotation in this case."
)

def type_cast_value(self, ctx: click.Context, value: t.Any) -> t.Any:
# get the result of a normal type_cast
Expand Down Expand Up @@ -72,9 +67,7 @@ def type_cast_value(self, ctx: click.Context, value: t.Any) -> t.Any:
)


def one_use_option(
*args: t.Any, type_annotation: type | str, **kwargs: t.Any
) -> t.Callable[[C], C]:
def one_use_option(*args: t.Any, **kwargs: t.Any) -> t.Callable[[C], C]:
"""
Wrapper of the click.option decorator that replaces any instances of
the Option class with the custom OneUseOption class
Expand All @@ -94,7 +87,6 @@ def one_use_option(

# use our OneUseOption class instead of a normal Option
kwargs["cls"] = OneUseOption
kwargs["type_annotation"] = type_annotation

# if dealing with a flag, switch to a counting option,
# and then assert if the count is not greater than 1 and cast to a bool
Expand Down
18 changes: 9 additions & 9 deletions tests/unit/param_classes/test_one_use_option.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

def test_one_use_option_multiple_allows_unused(runner):
@click.command()
@click.option("--foo", multiple=True, cls=OneUseOption, type_annotation=str)
@click.option("--foo", multiple=True, cls=OneUseOption)
def testcmd(foo):
click.echo(foo)

Expand All @@ -17,7 +17,7 @@ def testcmd(foo):

def test_one_use_option_multiple_allows_single_use(runner):
@click.command()
@click.option("--foo", multiple=True, cls=OneUseOption, type_annotation=str)
@click.option("--foo", multiple=True, cls=OneUseOption)
def testcmd(foo):
click.echo(foo)

Expand All @@ -28,7 +28,7 @@ def testcmd(foo):

def test_one_use_option_multiple_rejects_opt_used_twice(runner):
@click.command()
@click.option("--foo", multiple=True, cls=OneUseOption, type_annotation=str)
@click.option("--foo", multiple=True, cls=OneUseOption)
def testcmd(foo):
click.echo(foo)

Expand All @@ -39,7 +39,7 @@ def testcmd(foo):

def test_one_use_option_count_allows_unused(runner):
@click.command()
@click.option("--foo", count=True, cls=OneUseOption, type_annotation=str)
@click.option("--foo", count=True, cls=OneUseOption)
def testcmd(foo):
click.echo(foo)

Expand All @@ -50,7 +50,7 @@ def testcmd(foo):

def test_one_use_option_count_allows_used_once(runner):
@click.command()
@click.option("--foo", count=True, cls=OneUseOption, type_annotation=str)
@click.option("--foo", count=True, cls=OneUseOption)
def testcmd(foo):
click.echo(foo)

Expand All @@ -61,7 +61,7 @@ def testcmd(foo):

def test_one_use_option_count_rejects_opt_used_twice(runner):
@click.command()
@click.option("--foo", count=True, cls=OneUseOption, type_annotation=str)
@click.option("--foo", count=True, cls=OneUseOption)
def testcmd(foo):
click.echo(foo)

Expand All @@ -72,7 +72,7 @@ def testcmd(foo):

def test_one_use_option_fails_if_neither_count_nor_multiple(runner):
@click.command()
@click.option("--foo", cls=OneUseOption, type_annotation=str)
@click.option("--foo", cls=OneUseOption)
def testcmd(foo):
click.echo(foo)

Expand All @@ -86,9 +86,9 @@ def testcmd(foo):
@pytest.mark.parametrize("kwargs", ({"multiple": True}, {"count": True}))
def test_one_use_option_decorator_rejects_params(kwargs):
with pytest.raises(ValueError, match="cannot be used with multiple or count"):
one_use_option("--foo", type_annotation=str, **kwargs)
one_use_option("--foo", **kwargs)


def test_one_use_option_decorator_rejects_alternate_cls():
with pytest.raises(ValueError, match="cannot overwrite cls"):
one_use_option("--foo", cls=click.Option, type_annotation=str)
one_use_option("--foo", cls=click.Option)

0 comments on commit 2af5484

Please sign in to comment.