-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: support PEP604 syntax str | None
#168
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import collections | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import inspect | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import sys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import typing | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from enum import Enum | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from functools import partial | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -11,7 +12,17 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import Union | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import cast | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
UnionType = TypeVar("UnionType", bound="Union") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing_extensions import TypeAlias | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if sys.version_info >= (3, 10): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from types import UnionType | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# NB: `types.UnionType`, available since Python 3.10, is **not** a `type`, but is a class. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# We declare an empty class here to use in the instance checks below. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class UnionType: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
EnumType = TypeVar("EnumType", bound="Enum") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# conceptually bound to "Literal" but that's not valid in the spec | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# see: https://peps.python.org/pep-0586/#illegal-parameters-for-literal-at-type-check-time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -73,9 +84,52 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def _is_optional(type_: type) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Returns true if type_ is optional""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return typing.get_origin(type_) is Union and type(None) in typing.get_args(type_) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# NB: since `_GenericAlias` is a private attribute of the `typing` module, mypy doesn't find it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TypeAnnotation: TypeAlias = Union[type, typing._GenericAlias, UnionType] # type: ignore[name-defined] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
A function parameter's type annotation may be any of the following: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1) `type`, when declaring any of the built-in Python types | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
2) `typing._GenericAlias`, when declaring generic collection types or union types using pre-PEP | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
585 and pre-PEP 604 syntax (e.g. `List[int]`, `Optional[int]`, or `Union[int, None]`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
3) `types.UnionType`, when declaring union types using PEP604 syntax (e.g. `int | None`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
4) `types.GenericAlias`, when declaring generic collection types using PEP 585 syntax (e.g. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
`list[int]`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
`types.GenericAlias` is a subclass of `type`, but `typing._GenericAlias` and `types.UnionType` are | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
not and must be considered explicitly. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# TODO When dropping support for Python 3.9, deprecate this in favor of performing instance checks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# directly on the `TypeAnnotation` union type. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# NB: since `_GenericAlias` is a private attribute of the `typing` module, mypy doesn't find it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TYPE_ANNOTATION_TYPES = (type, typing._GenericAlias, UnionType) # type: ignore[attr-defined] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue as above, we should include
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def _is_optional(dtype: TypeAnnotation) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Check if a type is `Optional`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
An optional type may be declared using three syntaxes: `Optional[T]`, `Union[T, None]`, or `T | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
None`. All of these syntaxes is supported by this function. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Args: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dtype: A type. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Returns: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
True if the type is a union type with exactly two elements, one of which is `None`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
False otherwise. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Raises: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TypeError: If the input is not a valid `TypeAnnotation` type (see above). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+108
to
+118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not isinstance(dtype, TYPE_ANNOTATION_TYPES): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
raise TypeError(f"Expected type annotation, got {type(dtype)}: {dtype}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
origin = typing.get_origin(dtype) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
args = typing.get_args(dtype) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+122
to
+123
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit I think it's more in line with our convention to import these functions instead of referencing them as module attributes
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Optional[T] has `typing.Union` as its origin, but PEP604 syntax (e.g. `int | None`) has | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# `types.UnionType` as its origin. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
origin is not None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
and (origin is Union or origin is UnionType) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
and len(args) == 2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
and type(None) in args | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def _make_union_parser_worker( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -84,7 +138,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
value: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) -> UnionType: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Worker function behind union parsing. Iterates through possible parsers for the union and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
returns the value produced by the first parser that works. Otherwise raises an error if none | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
returns the value produced by the first parser that works. Otherwise, raises an error if none | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
work""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Need to do this in the case of type Optional[str], because otherwise it'll return the string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# 'None' instead of the object None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,11 +1,28 @@ | ||||||||||||||||||||||||
import sys | ||||||||||||||||||||||||
from typing import Iterable | ||||||||||||||||||||||||
from typing import List | ||||||||||||||||||||||||
from typing import Optional | ||||||||||||||||||||||||
from typing import Sequence | ||||||||||||||||||||||||
from typing import Union | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
from fgpyo.util import types | ||||||||||||||||||||||||
from fgpyo.util.inspect import NoneType | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
def test_is_listlike() -> None: | ||||||||||||||||||||||||
assert types.is_list_like(List[str]) | ||||||||||||||||||||||||
assert types.is_list_like(Iterable[str]) | ||||||||||||||||||||||||
assert types.is_list_like(Sequence[str]) | ||||||||||||||||||||||||
assert not types.is_list_like(str) | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
def test_is_optional() -> None: | ||||||||||||||||||||||||
assert types._is_optional(Union[str, NoneType]) | ||||||||||||||||||||||||
assert types._is_optional(Optional[str]) | ||||||||||||||||||||||||
assert not types._is_optional(str) | ||||||||||||||||||||||||
Comment on lines
+21
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion Test a couple union types that don't meet our criteria
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NB: for brevity (and to catch multiple failures in a single |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
if sys.version_info >= (3, 10): | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
def test_is_optional_python_310() -> None: | ||||||||||||||||||||||||
assert types._is_optional(str | None) | ||||||||||||||||||||||||
Comment on lines
+22
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion Save a couple lines?
Suggested change
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue
GenericAlias
should be included as a valid type annotation type. Also addfrom types import GenericAlias
above