From ef66fbbb661b792a20b79d6538abd4e7584ab496 Mon Sep 17 00:00:00 2001 From: Ted Brookings Date: Fri, 15 Mar 2024 13:20:07 -0400 Subject: [PATCH] Added a test, made a few things private --- fgpyo/util/inspect.py | 35 +++++++++++++++--------- fgpyo/util/metric.py | 2 +- fgpyo/util/tests/test_inspect.py | 46 ++++++++++++++++++++++---------- 3 files changed, 55 insertions(+), 28 deletions(-) diff --git a/fgpyo/util/inspect.py b/fgpyo/util/inspect.py index 367148a7..2056ce57 100644 --- a/fgpyo/util/inspect.py +++ b/fgpyo/util/inspect.py @@ -11,7 +11,7 @@ from typing import Type from typing import Union -if sys.version_info >= (3, 12): +if sys.version_info >= (3, 10): from typing import TypeAlias else: from typing_extensions import TypeAlias @@ -85,9 +85,18 @@ def is_attr_class(cls: type) -> bool: # type: ignore[arg-type] return hasattr(cls, "__attrs_attrs__") -MISSING_OR_NONE = {*MISSING, None} -DataclassesOrAttrClass = Union[DataclassesProtocol, AttrsInstance] +_MISSING_OR_NONE = {*MISSING, None} +"""Set of values that are considered missing or None for dataclasses or attr classes""" +_DataclassesOrAttrClass: TypeAlias = Union[DataclassesProtocol, AttrsInstance] +""" +TypeAlias for dataclasses or attr classes. Mostly nonsense because they are not true types, they +are traits, but there is no python trait-tester. +""" FieldType: TypeAlias = Union[dataclasses.Field, Attribute] +""" +TypeAlias for dataclass Fields or attrs Attributes. It will correspond to the correct type for the +corresponding _DataclassesOrAttrClass +""" def get_dataclasses_fields_dict( @@ -378,7 +387,7 @@ def get_parser() -> partial: def get_fields_dict( - cls: Union[DataclassesOrAttrClass, Type[DataclassesOrAttrClass]] + cls: Union[_DataclassesOrAttrClass, Type[_DataclassesOrAttrClass]] ) -> Mapping[str, FieldType]: """Get the fields dict from either a dataclasses or attr dataclass (or instance)""" if is_dataclasses_class(cls): @@ -390,7 +399,7 @@ def get_fields_dict( def get_fields( - cls: Union[DataclassesOrAttrClass, Type[DataclassesOrAttrClass]] + cls: Union[_DataclassesOrAttrClass, Type[_DataclassesOrAttrClass]] ) -> Tuple[FieldType, ...]: """Get the fields tuple from either a dataclasses or attr dataclass (or instance)""" if is_dataclasses_class(cls): @@ -401,15 +410,15 @@ def get_fields( raise TypeError("cls must a dataclasses or attr class") -# TypeVar to allow attr_from to be used with either an attr class or a dataclasses class -AttrFromType = TypeVar("AttrFromType") +_AttrFromType = TypeVar("_AttrFromType") +"""TypeVar to allow attr_from to be used with either an attr class or a dataclasses class""" def attr_from( - cls: Type[AttrFromType], + cls: Type[_AttrFromType], kwargs: Dict[str, str], parsers: Optional[Dict[type, Callable[[str], Any]]] = None, -) -> AttrFromType: +) -> _AttrFromType: """Builds an attr or dataclasses class from key-word arguments Args: @@ -455,7 +464,7 @@ def attr_from( set_value ), f"Do not know how to convert string to {attribute.type} for value: {str_value}" else: # no value, check for a default - assert attribute.default is not None or attribute_is_optional( + assert attribute.default is not None or _attribute_is_optional( attribute ), f"No value given and no default for attribute `{attribute.name}`" return_value = attribute.default @@ -468,13 +477,13 @@ def attr_from( return cls(**return_values) -def attribute_is_optional(attribute: FieldType) -> bool: +def _attribute_is_optional(attribute: FieldType) -> bool: """Returns True if the attribute is optional, False otherwise""" return typing.get_origin(attribute.type) is Union and isinstance( None, typing.get_args(attribute.type) ) -def attribute_has_default(attribute: FieldType) -> bool: +def _attribute_has_default(attribute: FieldType) -> bool: """Returns True if the attribute has a default value, False otherwise""" - return attribute.default not in MISSING_OR_NONE or attribute_is_optional(attribute) + return attribute.default not in _MISSING_OR_NONE or _attribute_is_optional(attribute) diff --git a/fgpyo/util/metric.py b/fgpyo/util/metric.py index b3776209..e9b91eee 100644 --- a/fgpyo/util/metric.py +++ b/fgpyo/util/metric.py @@ -190,7 +190,7 @@ def read(cls, path: Path, ignore_extra_fields: bool = True) -> Iterator[Any]: fields_with_defaults = [ field for field in missing_from_file - if inspect.attribute_has_default(field_name_to_attribute[field]) + if inspect._attribute_has_default(field_name_to_attribute[field]) ] # remove optional class fields from the fields missing_from_file = missing_from_file.difference(fields_with_defaults) diff --git a/fgpyo/util/tests/test_inspect.py b/fgpyo/util/tests/test_inspect.py index 56f110d3..dca5ab93 100644 --- a/fgpyo/util/tests/test_inspect.py +++ b/fgpyo/util/tests/test_inspect.py @@ -1,3 +1,4 @@ +import dataclasses from typing import Dict from typing import List from typing import Optional @@ -7,12 +8,14 @@ import attr import pytest +from fgpyo.util.inspect import _attribute_has_default +from fgpyo.util.inspect import _attribute_is_optional from fgpyo.util.inspect import attr_from -from fgpyo.util.inspect import attribute_has_default -from fgpyo.util.inspect import attribute_is_optional from fgpyo.util.inspect import dict_parser from fgpyo.util.inspect import get_fields from fgpyo.util.inspect import get_fields_dict +from fgpyo.util.inspect import is_attr_class +from fgpyo.util.inspect import is_dataclasses_class from fgpyo.util.inspect import list_parser from fgpyo.util.inspect import set_parser from fgpyo.util.inspect import tuple_parser @@ -44,22 +47,22 @@ def test_attr_from() -> None: def test_attribute_is_optional() -> None: fields_dict = attr.fields_dict(Name) - assert not attribute_is_optional(fields_dict["required"]) - assert not attribute_is_optional(fields_dict["custom_parser"]) - assert not attribute_is_optional(fields_dict["converted"]) - assert attribute_is_optional(fields_dict["optional_no_default"]) - assert attribute_is_optional(fields_dict["optional_with_default_none"]) - assert attribute_is_optional(fields_dict["optional_with_default_some"]) + assert not _attribute_is_optional(fields_dict["required"]) + assert not _attribute_is_optional(fields_dict["custom_parser"]) + assert not _attribute_is_optional(fields_dict["converted"]) + assert _attribute_is_optional(fields_dict["optional_no_default"]) + assert _attribute_is_optional(fields_dict["optional_with_default_none"]) + assert _attribute_is_optional(fields_dict["optional_with_default_some"]) def test_attribute_has_default() -> None: fields_dict = attr.fields_dict(Name) - assert not attribute_has_default(fields_dict["required"]) - assert not attribute_has_default(fields_dict["custom_parser"]) - assert not attribute_has_default(fields_dict["converted"]) - assert attribute_has_default(fields_dict["optional_no_default"]) - assert attribute_has_default(fields_dict["optional_with_default_none"]) - assert attribute_has_default(fields_dict["optional_with_default_some"]) + assert not _attribute_has_default(fields_dict["required"]) + assert not _attribute_has_default(fields_dict["custom_parser"]) + assert not _attribute_has_default(fields_dict["converted"]) + assert _attribute_has_default(fields_dict["optional_no_default"]) + assert _attribute_has_default(fields_dict["optional_with_default_none"]) + assert _attribute_has_default(fields_dict["optional_with_default_some"]) class Foo: @@ -71,6 +74,11 @@ class Bar: foo: Foo +@dataclasses.dataclass(frozen=True) +class Baz: + foo: Foo + + # Test for regression #94 - the call to attr_from succeeds when the check for None type # in inspect._get_parser is done incorrectly. def test_attr_from_custom_type_without_parser_fails() -> None: @@ -125,3 +133,13 @@ class NonDataClass: with pytest.raises(TypeError): attr_from(cls=NonDataClass, kwargs={"x": "1"}, parsers={int: int}) + + +def test_is_attrs_is_dataclasses() -> None: + assert not is_attr_class(Foo) + assert not is_dataclasses_class(Foo) + + assert is_attr_class(Bar) + assert not is_dataclasses_class(Bar) + assert is_dataclasses_class(Baz) + assert not is_attr_class(Baz)