anntzer / defopt

Effortless argument parser
https://pypi.org/project/defopt/
MIT License
213 stars 11 forks source link

When subclassing enum with data mixin, choices are not present on CLI #106

Closed Ben-Habermeyer closed 2 years ago

Ben-Habermeyer commented 2 years ago

Hello! I am subclassing enum https://docs.python.org/3/library/enum.html#restricted-enum-subclassing with a custom data type and see that when using defopt.run with an Enum type that the data mixin is shown as the type, not the enum itself.

e.g. the following code snippet, using latest 6.3.0

from enum import Enum
from typing import NamedTuple, Union
import defopt

class Data(NamedTuple):
    gene1: str
    gene2: str

class SpecialCase(Data, Enum):
    SMA = Data(gene1="SMN1", gene2="SMN2")

def myfun(*, case: SpecialCase):
    """print stuff

    Args:
        case: special case
    """
    print(case.name)
    print(case.gene1)
    print(case.gene2 == "SMN2")

if __name__ == "__main__":
    defopt.run(myfun)

On the cli for this, say test.py I see that the NamedTuple data mixin is used but not the Enum

usage: test.py [-h] -c gene1 gene2

print stuff

optional arguments:
  -h, --help            show this help message and exit
  -c gene1 gene2, --case gene1 gene2
                        special case

This is problematic since it should actually be an Enum. A workaround is to define the type of case in myfun to be Union[Enum, SpecialCase] in which the it only accepts one argument, but does not show the possibilities

usage: test.py [-h] -c CASE

print stuff

optional arguments:
  -h, --help            show this help message and exit
  -c CASE, --case CASE  special case

Without the Data mixin class, the enum choices appear correctly

usage: test2.py [-h] -c {SMA}

print stuff

optional arguments:
  -h, --help            show this help message and exit
  -c {SMA}, --case {SMA}
                        special case

Since type(SpecialCase) is actually enum.EnumMeta and not Data I think this is an issue with the parsing.

I appreciate your work on defopt and thank you in advance for looking into this!

anntzer commented 2 years ago

Seems reasonable. Can you confirm that

diff --git i/lib/defopt.py w/lib/defopt.py
index 16898dc..a71e38f 100644
--- i/lib/defopt.py
+++ w/lib/defopt.py
@@ -491,7 +491,12 @@ def _populate_parser(func, parser, parsers, short, cli_options,
                 kwargs['action'] = 'append'
                 kwargs['default'] = _DefaultList()
         member_types = None
-        if _ti_get_origin(type_) is tuple:
+
+        if isinstance(type_, type) and issubclass(type_, Enum):
+            # Enums must be checked first to handle enums-of-namedtuples.
+            kwargs['type'] = _get_parser(type_, parsers)
+            kwargs['metavar'] = '{' + ','.join(type_.__members__) + '}'
+        elif _ti_get_origin(type_) is tuple:
             member_types = _ti_get_args(type_)
             # Variable-length tuples of homogenous type are specified like
             # Tuple[int, ...]
@@ -516,9 +521,7 @@ def _populate_parser(func, parser, parsers, short, cli_options,
                 kwargs['metavar'] = type_._fields
         else:
             kwargs['type'] = _get_parser(type_, parsers)
-            if isinstance(type_, type) and issubclass(type_, Enum):
-                kwargs['metavar'] = '{' + ','.join(type_.__members__) + '}'
-            elif _ti_get_origin(type_) is Literal:
+            if _ti_get_origin(type_) is Literal:
                 kwargs['metavar'] = (
                     '{' + ','.join(map(str, _ti_get_args(type_))) + '}')
         actions.append(_add_argument(parser, name, short, **kwargs))

fixes the issue for you?

Ben-Habermeyer commented 2 years ago

@anntzer yes this looks right to me!

›› python test2.py -h                                                                                                           10:59 Mar14
6.3.0
usage: test.py [-h] -c {SMA,RHD}

print stuff

optional arguments:
  -h, --help            show this help message and exit
  -c {SMA,RHD}, --case {SMA,RHD}
                        special case
anntzer commented 2 years ago

OK, pushed the patch.

Ben-Habermeyer commented 2 years ago

Thank you @anntzer ! I will look for the new release on pypi/conda

anntzer commented 2 years ago

That will probably not happen before a couple of weeks in all likelihood, due to https://github.com/anntzer/defopt/issues/107#issuecomment-1067590901.

Ben-Habermeyer commented 2 years ago

Hi @anntzer following up here - is there a plan to include this change in a pypi release?

anntzer commented 2 years ago

Done :) Thanks for the ping.

Ben-Habermeyer commented 2 years ago

Done :) Thanks for the ping.

Sweet! Thanks again :)