lebrice / SimpleParsing

Simple, Elegant, Typed Argument Parsing with argparse
MIT License
410 stars 52 forks source link

Postponed annotation bug #168

Closed mixilchenko closed 1 year ago

mixilchenko commented 2 years ago
# file a.py
from __future__ import annotations
from dataclasses import dataclass
from pathlib import Path

@dataclass
class A:
    p: Path | None
# file b.py
from dataclasses import dataclass
from simple_parsing import ArgumentParser
from a import A

@dataclass
class B(A):
    v: int

if __name__ == '__main__':
    parser = ArgumentParser()
    parser.add_arguments(B, 'b')
    args = parser.parse_args()
    print(args.b)

Running python b.py -h gives an error

Traceback (most recent call last):
  File ".../b.py", line 15, in <module>
    parser.add_arguments(B, 'b')
  File ".../lib/python3.9/site-packages/simple_parsing/parsing.py", line 247, in add_arguments
    new_wrapper = dataclass_wrapper_class(dataclass, dest, prefix=prefix, default=default)
  File ".../lib/python3.9/site-packages/simple_parsing/wrappers/dataclass_wrapper.py", line 60, in __init__
    field_type = get_field_type_from_annotations(self.dataclass, field.name)
  File ".../lib/python3.9/site-packages/simple_parsing/annotation_utils/get_field_annotations.py", line 163, in get_field_type_from_annotations
    annotations_dict = get_type_hints(some_class, localns=local_ns, globalns=global_ns)
  File ".../lib/python3.9/typing.py", line 1458, in get_type_hints
    value = _eval_type(value, base_globals, localns)
  File ".../lib/python3.9/typing.py", line 292, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File ".../lib/python3.9/typing.py", line 554, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'Path' is not defined
mixilchenko commented 2 years ago

Importing Path in b.py is a workaround

mixilchenko commented 2 years ago

I guess here we should also iterate through class mro and add globals from base class modules

@lebrice I can fix it if this solution is fine for you Also for now I can't imagine how to write test for this use case.

lebrice commented 2 years ago

Hey @mixilchenko, thanks for posting this.

Yeah that solution makes sense to me.

There are some small complications that might arise, but that we can probably ignore for now. For example, if in the module of the base class, some annotation e.g. Foo refers to some class Foo, while in the scope of the other module, the same name refers to another class Foo, then the first Foo class should be used.

Feel free to make a PR for this, if you have a fix in mind, I'd be very grateful :)

lebrice commented 2 years ago

As for tests, I suggest creating a folder under test with these exact modules.