Perf improvements to #268 (additionalProperties Union Fix):

- Added CHANGELOG entry
- Change inner_properties_without_template to bool generated once
- Change inner_properties_with_template to generator

Author: dbanty

CHANGELOG.md

@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## 0.7.3 - Unreleased
+### Fixes
+- Spacing and extra returns for Union types of `additionalProperties` (#266 & #268). Thanks @joshzana & @packyg!
+
 ## 0.7.2 - 2020-12-08
 ### Fixes
 - A bug in handling optional properties that are themselves models (introduced in 0.7.1) (#262). Thanks @packyg!

end_to_end_tests/golden-record-custom/custom_e2e/models/model_with_any_json_properties.py

@@ -58,7 +58,7 @@ def _parse_additional_property(
                     return additional_property
                 except:  # noqa: E722
                     pass
-                return cast(str, data)
+                return cast(Union[ModelWithAnyJsonPropertiesAdditionalProperty, List[str], str, float, int, bool], data)
 
             additional_property = _parse_additional_property(prop_dict)
 

end_to_end_tests/golden-record/my_test_api_client/models/model_with_any_json_properties.py

@@ -58,7 +58,7 @@ def _parse_additional_property(
                     return additional_property
                 except:  # noqa: E722
                     pass
-                return cast(str, data)
+                return cast(Union[ModelWithAnyJsonPropertiesAdditionalProperty, List[str], str, float, int, bool], data)
 
             additional_property = _parse_additional_property(prop_dict)
 

openapi_python_client/__init__.py

@@ -25,8 +25,15 @@
 __version__ = version(__package__)
 
 
+TEMPLATE_FILTERS = {
+    "snakecase": utils.snake_case,
+    "kebabcase": utils.kebab_case,
+    "pascalcase": utils.pascal_case,
+    "any": any,
+}
+
+
 class Project:
-    TEMPLATE_FILTERS = {"snakecase": utils.snake_case, "kebabcase": utils.kebab_case, "pascalcase": utils.pascal_case}
     project_name_override: Optional[str] = None
     package_name_override: Optional[str] = None
     package_version_override: Optional[str] = None
@@ -57,7 +64,7 @@ def __init__(self, *, openapi: GeneratorData, custom_template_path: Optional[Pat
         )
         self.version: str = self.package_version_override or openapi.version
 
-        self.env.filters.update(self.TEMPLATE_FILTERS)
+        self.env.filters.update(TEMPLATE_FILTERS)
 
     def build(self) -> Sequence[GeneratorError]:
         """ Create the project from templates """

openapi_python_client/parser/properties/__init__.py

@@ -1,18 +1,17 @@
-import builtins
 from itertools import chain
-from typing import Any, ClassVar, Dict, Generic, Iterable, List, Optional, Set, Tuple, TypeVar, Union
+from typing import Any, ClassVar, Dict, Generic, Iterable, List, Optional, Set, Tuple, TypeVar, Union, Iterator
 
 import attr
 
-from ... import schema as oai
-from ... import utils
-from ..errors import PropertyError, ValidationError
-from ..reference import Reference
 from .converter import convert, convert_chain
 from .enum_property import EnumProperty
 from .model_property import ModelProperty
 from .property import Property
 from .schemas import Schemas
+from ..errors import PropertyError, ValidationError
+from ..reference import Reference
+from ... import schema as oai
+from ... import utils
 
 
 @attr.s(auto_attribs=True, frozen=True)
@@ -160,6 +159,13 @@ class UnionProperty(Property):
 
     inner_properties: List[Property]
     template: ClassVar[str] = "union_property.pyi"
+    has_properties_without_templates: bool = attr.ib(init=False)
+
+    def __attrs_post_init__(self) -> None:
+        super().__attrs_post_init__()
+        object.__setattr__(
+            self, "has_properties_without_templates", any(prop.template is None for prop in self.inner_properties)
+        )
 
     def get_type_string(self, no_optional: bool = False) -> str:
         """ Get a string representation of type that should be used when declaring this property """
@@ -185,16 +191,11 @@ def get_imports(self, *, prefix: str) -> Set[str]:
         imports = super().get_imports(prefix=prefix)
         for inner_prop in self.inner_properties:
             imports.update(inner_prop.get_imports(prefix=prefix))
-        imports.add("from typing import Union")
+        imports.add("from typing import cast, Union")
         return imports
 
-    @builtins.property
-    def inner_properties_with_template(self) -> List[Property]:
-        return [prop for prop in self.inner_properties if prop.template]
-
-    @builtins.property
-    def inner_properties_without_template(self) -> List[Property]:
-        return [prop for prop in self.inner_properties if not prop.template]
+    def inner_properties_with_template(self) -> Iterator[Property]:
+        return (prop for prop in self.inner_properties if prop.template)
 
 
 def _string_based_property(

openapi_python_client/templates/property_templates/union_property.pyi

@@ -2,8 +2,8 @@
 def _parse_{{ property.python_name }}(data: Any) -> {{ property.get_type_string() }}:
     data = None if isinstance(data, Unset) else data
     {{ property.python_name }}: {{ property.get_type_string() }}
-    {% for inner_property in property.inner_properties_with_template %}
-    {% if not loop.last or property.inner_properties_without_template %}
+    {% for inner_property in property.inner_properties_with_template() %}
+    {% if not loop.last or property.has_properties_without_templates %}
     try:
     {% from "property_templates/" + inner_property.template import construct %}
         {{ construct(inner_property, "data", initial_value="UNSET") | indent(8) }}
@@ -16,9 +16,9 @@ def _parse_{{ property.python_name }}(data: Any) -> {{ property.get_type_string(
     return {{ property.python_name }}
     {% endif %}
     {% endfor %}
-    {% if property.inner_properties_without_template %}
+    {% if property.has_properties_without_templates %}
     {# Doesn't really matter what we cast it to as this type will be erased, so cast to one of the options #}
-    return cast({{ property.inner_properties_without_template[0].get_type_string() }}, data)
+    return cast({{ property.get_type_string() }}, data)
     {% endif %}
 
 {{ property.python_name }} = _parse_{{ property.python_name }}({{ source }})
@@ -32,28 +32,28 @@ if isinstance({{ source }}, Unset):
     {{ destination }} = UNSET
 {% endif %}
 {% if property.nullable %}
-{% if property.required %}
+    {% if property.required %}
 if {{ source }} is None:
-{% else %}{# There's an if UNSET statement before this #}
+    {% else %}{# There's an if UNSET statement before this #}
 elif {{ source }} is None:
-{% endif %}
+    {% endif %}
     {{ destination }}{% if declare_type %}: {{ property.get_type_string() }}{% endif %} = None
 {% endif %}
-{% for inner_property in property.inner_properties_with_template %}
+{% for inner_property in property.inner_properties_with_template() %}
     {% if loop.first and property.required and not property.nullable %}{# No if UNSET or if None statement before this #}
 if isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}):
-    {% elif not loop.last or property.inner_properties_without_template %}
+    {% elif not loop.last or property.has_properties_without_templates %}
 elif isinstance({{ source }}, {{ inner_property.get_instance_type_string() }}):
     {% else %}
 else:
     {% endif %}
-{% from "property_templates/" + inner_property.template import transform %}
+    {% from "property_templates/" + inner_property.template import transform %}
     {{ transform(inner_property, source, destination, declare_type=False) | indent(4) }}
 {% endfor %}
-{% if property.inner_properties_without_template and (property.inner_properties_with_template or not property.required)%}
+{% if property.has_properties_without_templates and (property.inner_properties_with_template() | any or not property.required)%}
 else:
     {{ destination }} = {{ source }}
-{% elif property.inner_properties_without_template %}
+{% elif property.has_properties_without_templates %}
 {{ destination }} = {{ source }}
 {% endif %}