lebrice / SimpleParsing

Simple, Elegant, Typed Argument Parsing with argparse
MIT License
386 stars 47 forks source link

Future annotations cause add_arguments "NameError" when using Serializable #181

Closed zhiruiluo closed 1 year ago

zhiruiluo commented 1 year ago

Describe the bug the from __future__ import annotations cause add_arguments "NameError" when using Serializable It will raise the following error: "NameError: name 'D' is not defined"

To Reproduce

# test_simple_parsing.py
from __future__ import annotations
from simple_parsing import Serializable, ArgumentParser, subgroups
from dataclasses import dataclass
import pytest

@dataclass
class MyArguments(Serializable):
    arg1: str = 'this_argment'

@pytest.mark.parametrize(
    'sys_argv, result', [
        (['test.py'], 'this_argment'),
        (['test.py', '--arg1', 'test2'], 'test2')
    ],
)
def test_simple_parsing(sys_argv, result):
    parser = ArgumentParser()
    parser.add_arguments(MyArguments, 'myargs')
    args, _ = parser.parse_known_args(sys_argv)
    assert args.myargs.arg1 == result

Expected behavior A clear and concise description of what you expected to happen.

$ pytest test_simple_parsing.py
2 passed

Actual behavior A clear and concise description of what is happening.

$ pytest test_simple_parsing.py
2 failed
NameError: name 'D' is not defined

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

zhiruiluo commented 1 year ago

Absolutely, we will pass the test by removing the from __future__ import annotations from this file.

zhiruiluo commented 1 year ago

Here is my proposed fix #182. Added an entry to the forward_refs_to_types in get_field_annotation.py to fix the error. Added a test_issue_181.py file about this issue.

zhiruiluo commented 1 year ago

Looks like that this is related to #168.

zhiruiluo commented 1 year ago

Thanks for review my code, @lebrice. The fix is enclosed in #183. I am closing this issue now.