Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

from __future__ import annotations support #120

Closed
carmocca opened this issue Feb 1, 2022 · 5 comments · Fixed by #294
Closed

from __future__ import annotations support #120

carmocca opened this issue Feb 1, 2022 · 5 comments · Fixed by #294
Labels
enhancement New feature or request

Comments

@carmocca
Copy link
Contributor

carmocca commented Feb 1, 2022

These 2 should be equal on Python>=3.7

from typing import Optional

from jsonargparse import ArgumentParser


class Foo:
    def __init__(self, x: Optional[str] = None) -> None:
        self.x = x

parser = ArgumentParser()
parser.add_class_arguments(Foo)
args = parser.parse_args()
print(args)  # Namespace(x=None)
from __future__ import annotations

from jsonargparse import ArgumentParser


class Foo:
    def __init__(self, x: str | None = None) -> None:
        self.x = x

parser = ArgumentParser()
parser.add_class_arguments(Foo)
args = parser.parse_args()
print(args)  # Namespace()

Additional context

We want to adopt this in Lightning: Lightning-AI/pytorch-lightning#11205

@mauvilsa
Copy link
Member

mauvilsa commented Feb 2, 2022

Some support for future annotations in python 3.7-3.9 could be added. However, your main motivation for it is the use of the new syntax for unions using a vertical bar. I don't see support for this being added any time soon. With PEP 563 by postponing the evaluation of annotations, the import and use of the module does not fail because the annotations become strings. But jsonargparse would need to evaluate them at runtime. And that syntax is not a valid expression in python 3.7-3.9, so there is no native way to evaluate it. The problem can be observed with the following:

from __future__ import annotations
from typing import get_type_hints

def my_func(param: int | str):
    ...

print(get_type_hints(my_func))

In python 3.7-3.9 this fails in the last line with TypeError: unsupported operand type(s) for |: 'type' and 'type'. Implementing a parser for the new syntax in jsonargparse would be out of the question. There is a new project that is trying to implement this parsing https://github.com/PrettyWood/future-typing. But it is still too immature to consider adding it as a dependency or even to assume that it will work correctly on most cases.

On the other hand, making PEP 563 the default was initially planned for python 3.10. But it was postponed because there is a bit of a controversy around this. A good summary can be found at https://lwn.net/Articles/858576/. Better to wait until it is more definite how things play out and there are more reliable tools for runtime evaluation. Particularly in the case of jsonargparse which does not yet have a community that would help in maintaining it.

@mauvilsa mauvilsa added the enhancement New feature or request label Feb 2, 2022
@carmocca
Copy link
Contributor Author

carmocca commented Feb 2, 2022

The Lightning issue linked above suggest adopting 3 PEPs together: PEP 585, PEP 604, and PEP 563.

Out of those, 585 and 563 are the most interesting. 585 to avoid users having to add the imports when their IDE auto-fills a method implementation and 563 to reduce circular dependencies.

If 604 and 563 cannot be supported yet here, I guess it means we cannot convert the code in Lightning yet.

Is there a problem with 585 though?

@mauvilsa
Copy link
Member

mauvilsa commented Feb 4, 2022

Is there a problem with 585 though?

It has the same problem, no native way to evaluate the type hint.

from __future__ import annotations
from typing import get_type_hints

def my_func(param: list[int]):
    ...

print(get_type_hints(my_func))

This produces TypeError: 'type' object is not subscriptable.

mauvilsa added a commit that referenced this issue Jun 6, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
mauvilsa added a commit that referenced this issue Jun 6, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
mauvilsa added a commit that referenced this issue Jun 6, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
mauvilsa added a commit that referenced this issue Jun 6, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
@mauvilsa
Copy link
Member

mauvilsa commented Jun 6, 2023

Some time ago I added backporting of PEPs 585 and 604 types in python 3.7-3.9, though this was only used to get types from stub (*.pyi) files. Even though it has been out and used for a while, bugs in the backport might no have been discovered, because failures are silent and the stub types simply not resolved.

PEP 563 is now implemented (see pull request #294) including the backports for python 3.7-3.9. All the unit tests are now run using future annotations. Nonetheless, there might be bugs which would be good to discover before including this in a release. @carmocca how about trying the pyupgrade of lightning in a branch and temporarily changing the requirement to jsonargparse[signatures] @ https://github.com/omni-us/jsonargparse/zipball/future-annotations to test things out?

@carmocca
Copy link
Contributor Author

carmocca commented Jun 6, 2023

Sure. You can open a PR on Lightning with the changes to let our CI run.

mauvilsa added a commit that referenced this issue Jun 7, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
mauvilsa added a commit that referenced this issue Jun 12, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).

NEEDS WORK! PENDING:
- tests failing for python 3.6
- try-except each get_arg_type so that only problematic parameters are not evaluated.
  * if failure return exception
  * do debug log for each failed parameter
  * if all parameters fail do a single debug log
mauvilsa added a commit that referenced this issue Jun 22, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).

NEEDS WORK! PENDING:
- tests failing in py36
mauvilsa added a commit that referenced this issue Jun 22, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).

NEEDS WORK! PENDING:
- tests failing in py36
mauvilsa added a commit that referenced this issue Jun 22, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
mauvilsa added a commit that referenced this issue Jun 22, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
mauvilsa added a commit that referenced this issue Jun 22, 2023
- Backport types in python<=3.9 to support PEP 585 and 604 for postponed evaluation of annotations (#120).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants