edornd / argdantic

Typed command line interfaces with argparse and pydantic
MIT License
38 stars 4 forks source link

Crash when `from __future__ import annotations` is used #44

Closed danielkza closed 3 weeks ago

danielkza commented 6 months ago

Describe the bug

To Reproduce Add from __future__ import annotations to the top of a Python file and use argdantic

Expected behavior No crash

Actual behaviour

venv/lib64/python3.12/site-packages/argdantic/core.py:145: in __call__
    self.entrypoint = self._build_entrypoint()
venv/lib64/python3.12/site-packages/argdantic/core.py:212: in _build_entrypoint
    self.commands[0].build(parser=parser)
venv/lib64/python3.12/site-packages/argdantic/core.py:88: in build
    tracker = argument.build(parser=parser)
venv/lib64/python3.12/site-packages/argdantic/parsing/arguments.py:113: in build
    cli_type, cli_metavar = cli_types.get(type_name(self.field_type), cli_default)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

field_type = ForwardRef('Path | None')

    def type_name(field_type: type) -> str:
        """Returns the name of the type, or the name of the type's origin, if the type is a
        typing construct.
        Args:
            field_type (type): pydantic field type
        Returns:
            str: name of the type
        """
        origin = types.get_origin(field_type)
        if origin is not None:
            name = origin.__name__
        else:
>           name = field_type.__name__
E           AttributeError: 'ForwardRef' object has no attribute '__name__'. Did you mean: '__ne__'?
edornd commented 6 months ago

Hi @danielkza, I see from your traceback that you're using python 3.12, that's most likely the issue. I'll try to add it to the test suite so that we can verify this issue and fix it.

danielkza commented 6 months ago

Python 3.12 works fine without the future annotations though! if that makes any difference. We prob need to resolve forwardrefs, I think there is something for it somewhere in pydantic: https://github.com/pydantic/pydantic/blob/b1bb24e8b7687e5891bce33f551625ac01b74b66/pydantic/v1/typing.py#L63

or maybe even take it up a layer further, and try to reuse some of the type evaluation logic if possible.

edornd commented 6 months ago

Thanks for the inputs, this is next in line to be solved since it's a bit critical. The link you sent is a good example of why I really never bothered with Python typing that much, so far 🙃 It's such a mess. Let's see what can be done though!

edornd commented 3 weeks ago

Just a few comments on this: I looked into this and the rabbit hole is deeper than I thought. As you know, the problem is basically linked to the lack of actual type annotations when using the __future__ import, which is the intended behavior for postponed annotations. This is however a huge issue for a library based on type annotations, as you can imagine. It is true however that pydantic just "works" with it, the only hiccup I found was in the initial manual wrapping I was carrying out.

I shoud have patched it (at least for the moment) by resolving types via typing.get_type_hints before inspecting functions, from there on it seems to be working fine since pydantic takes the lead. However there are some caveats linked to scopes, but I fear there's nothing I can do in that case, even pydantic has limitations there (see https://github.com/pydantic/pydantic/discussions/4521).