Skip to content

Fixes to union properties#241

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.

Sign up for GitHub

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

Merged
dbanty merged 6 commits into from
Nov 18, 2020
Merged

Fixes to union properties #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0e0525b
Fix indentation in union property template
packyg Nov 13, 2020
eb45e92
Fix for union property
packyg Nov 13, 2020
072f8da
Update templates construct to allow different initial value, use in u…
packyg Nov 13, 2020
7223731
Update e2e tests, add model with union prop
packyg Nov 13, 2020
25f758c
Handle when _parse<property> is given UNSET
packyg Nov 13, 2020
15f1284
Regen w/ custom template
packyg Nov 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion end_to_end_tests/golden-record-custom/custom_e2e/api/tests/defaults_tests_defaults_post.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ def httpx_request(

json_enum_prop: Union[Unset, AnEnum] = UNSET
if not isinstance(enum_prop, Unset):
json_enum_prop = enum_prop.value
json_enum_prop = enum_prop

params: Dict[str, Any] = {}
if string_prop is not UNSET:
Expand Down
1 change: 1 addition & 0 deletions end_to_end_tests/golden-record-custom/custom_e2e/models/__init__.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@
from .body_upload_file_tests_upload_post import BodyUploadFileTestsUploadPost
from .different_enum import DifferentEnum
from .http_validation_error import HTTPValidationError
from .model_with_union_property import ModelWithUnionProperty
from .test_inline_objectsjson_body import TestInlineObjectsjsonBody
from .test_inline_objectsresponse_200 import TestInlineObjectsresponse_200
from .validation_error import ValidationError
7 changes: 4 additions & 3 deletions end_to_end_tests/golden-record-custom/custom_e2e/models/a_model.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,15 +74,16 @@ def to_dict(self) -> Dict[str, Any]:
def from_dict(d: Dict[str, Any]) -> "AModel":
an_enum_value = AnEnum(d["an_enum_value"])

def _parse_a_camel_date_time(data: Dict[str, Any]) -> Union[datetime.datetime, datetime.date]:
def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.date]:
data = None if isinstance(data, Unset) else data
a_camel_date_time: Union[datetime.datetime, datetime.date]
try:
a_camel_date_time = isoparse(d["aCamelDateTime"])
a_camel_date_time = isoparse(data)

return a_camel_date_time
except: # noqa: E722
pass
a_camel_date_time = isoparse(d["aCamelDateTime"]).date()
a_camel_date_time = isoparse(data).date()

return a_camel_date_time

Expand Down
59 changes: 59 additions & 0 deletions end_to_end_tests/golden-record-custom/custom_e2e/models/model_with_union_property.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
from typing import Any, Dict, Union

import attr

from ..models.an_enum import AnEnum
from ..models.an_int_enum import AnIntEnum
from ..types import UNSET, Unset


@attr.s(auto_attribs=True)
class ModelWithUnionProperty:
""" """

a_property: Union[Unset, AnEnum, AnIntEnum] = UNSET

def to_dict(self) -> Dict[str, Any]:
a_property: Union[Unset, AnEnum, AnIntEnum]
if isinstance(self.a_property, Unset):
a_property = UNSET
elif isinstance(self.a_property, AnEnum):
a_property = UNSET
if not isinstance(self.a_property, Unset):
a_property = self.a_property

else:
a_property = UNSET
if not isinstance(self.a_property, Unset):
a_property = self.a_property

field_dict = {}
if a_property is not UNSET:
field_dict["a_property"] = a_property

return field_dict

@staticmethod
def from_dict(d: Dict[str, Any]) -> "ModelWithUnionProperty":
def _parse_a_property(data: Any) -> Union[Unset, AnEnum, AnIntEnum]:
data = None if isinstance(data, Unset) else data
a_property: Union[Unset, AnEnum, AnIntEnum]
try:
a_property = UNSET
if data is not None:
a_property = AnEnum(data)

return a_property
except: # noqa: E722
pass
a_property = UNSET
if data is not None:
a_property = AnIntEnum(data)

return a_property

a_property = _parse_a_property(d.get("a_property", UNSET))

return ModelWithUnionProperty(
a_property=a_property,
)
2 changes: 1 addition & 1 deletion end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -53,7 +53,7 @@ def _get_kwargs(

json_enum_prop: Union[Unset, AnEnum] = UNSET
if not isinstance(enum_prop, Unset):
json_enum_prop = enum_prop.value
json_enum_prop = enum_prop

params: Dict[str, Any] = {}
if string_prop is not UNSET:
Expand Down
1 change: 1 addition & 0 deletions end_to_end_tests/golden-record/my_test_api_client/models/__init__.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@
from .body_upload_file_tests_upload_post import BodyUploadFileTestsUploadPost
from .different_enum import DifferentEnum
from .http_validation_error import HTTPValidationError
from .model_with_union_property import ModelWithUnionProperty
from .test_inline_objectsjson_body import TestInlineObjectsjsonBody
from .test_inline_objectsresponse_200 import TestInlineObjectsresponse_200
from .validation_error import ValidationError
7 changes: 4 additions & 3 deletions end_to_end_tests/golden-record/my_test_api_client/models/a_model.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,15 +74,16 @@ def to_dict(self) -> Dict[str, Any]:
def from_dict(d: Dict[str, Any]) -> "AModel":
an_enum_value = AnEnum(d["an_enum_value"])

def _parse_a_camel_date_time(data: Dict[str, Any]) -> Union[datetime.datetime, datetime.date]:
def _parse_a_camel_date_time(data: Any) -> Union[datetime.datetime, datetime.date]:
data = None if isinstance(data, Unset) else data
a_camel_date_time: Union[datetime.datetime, datetime.date]
try:
a_camel_date_time = isoparse(d["aCamelDateTime"])
a_camel_date_time = isoparse(data)

return a_camel_date_time
except: # noqa: E722
pass
a_camel_date_time = isoparse(d["aCamelDateTime"]).date()
a_camel_date_time = isoparse(data).date()

return a_camel_date_time

Expand Down
59 changes: 59 additions & 0 deletions end_to_end_tests/golden-record/my_test_api_client/models/model_with_union_property.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
from typing import Any, Dict, Union

import attr

from ..models.an_enum import AnEnum
from ..models.an_int_enum import AnIntEnum
from ..types import UNSET, Unset


@attr.s(auto_attribs=True)
class ModelWithUnionProperty:
""" """

a_property: Union[Unset, AnEnum, AnIntEnum] = UNSET

def to_dict(self) -> Dict[str, Any]:
a_property: Union[Unset, AnEnum, AnIntEnum]
if isinstance(self.a_property, Unset):
a_property = UNSET
elif isinstance(self.a_property, AnEnum):
a_property = UNSET
if not isinstance(self.a_property, Unset):
a_property = self.a_property
Comment on lines +20 to +23

@dbantydbantyNov 14, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird, I'll try to find the place in the template that causes it. Obviously no need to double check isinstance on the same property, we already know it's AnEnum.

@dbantydbantyNov 14, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where your changes would have caused this, so it was probably an existing issue with Unions/UNSET (I know there is at least one issue related to that). The inner_property is probably being set as not required which makes the inner constructor redo the check. I can handle that in a future version.


else:
a_property = UNSET
if not isinstance(self.a_property, Unset):
a_property = self.a_property

field_dict = {}
if a_property is not UNSET:
field_dict["a_property"] = a_property

return field_dict

@staticmethod
def from_dict(d: Dict[str, Any]) -> "ModelWithUnionProperty":
def _parse_a_property(data: Any) -> Union[Unset, AnEnum, AnIntEnum]:
data = None if isinstance(data, Unset) else data

@dbantydbantyNov 14, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why None this instead of checking for Unset further down? Less template code when nullables are involved maybe?

@packygpackygNov 18, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this was to be able to reuse construct from template for the inner property without further modifying.

Most of the constructs just check if {{ source }} is not None and don't account for UNSETs.

I'll look a bit more to see if there is a better way.

@dbantydbantyNov 18, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's not an easy fix it's not a huge deal. If you want to just regen the custom template e2e test so the checks pass I'll merge this.

a_property: Union[Unset, AnEnum, AnIntEnum]
try:
a_property = UNSET
if data is not None:
a_property = AnEnum(data)

return a_property
except: # noqa: E722
pass
a_property = UNSET
if data is not None:
a_property = AnIntEnum(data)

return a_property

a_property = _parse_a_property(d.get("a_property", UNSET))

return ModelWithUnionProperty(
a_property=a_property,
)
12 changes: 12 additions & 0 deletions end_to_end_tests/openapi.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -727,6 +727,18 @@
"type": "string"
}
}
},
"ModelWithUnionProperty": {
"title": "ModelWithUnionProperty",
"type": "object",
"properties": {
"a_property": {
"oneOf": [
{"$ref": "#/components/schemas/AnEnum"},
{"$ref": "#/components/schemas/AnIntEnum"}
]
}
}
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions openapi_python_client/templates/property_templates/date_property.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
{% macro construct(property, source) %}
{% macro construct(property, source, initial_value="None") %}
{% if property.required %}
{{ property.python_name }} = isoparse({{ source }}).date()
{% else %}
{{ property.python_name }} = None
{{ property.python_name }} = {{ initial_value }}
if {{ source }} is not None:
{{ property.python_name }} = isoparse(cast(str, {{ source }})).date()
{% endif %}
{% endmacro %}

{% macro transform(property, source, destination) %}
{% macro transform(property, source, destination, declare_type=True) %}
{% if property.required %}
{% if property.nullable %}
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
{% else %}
{{ destination }} = {{ source }}.isoformat()
{% endif %}
{% else %}
{{ destination }}: Union[Unset, str] = UNSET
{{ destination }}{% if declare_type %}: Union[Unset, str]{% endif %} = UNSET
if not isinstance({{ source }}, Unset):
{% if property.nullable %}
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
Expand Down
8 changes: 4 additions & 4 deletions openapi_python_client/templates/property_templates/datetime_property.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
{% macro construct(property, source) %}
{% macro construct(property, source, initial_value="None") %}
{% if property.required %}
{{ property.python_name }} = isoparse({{ source }})
{% else %}
{{ property.python_name }} = None
{{ property.python_name }} = {{ initial_value }}
if {{ source }} is not None:
{{ property.python_name }} = isoparse(cast(str, {{ source }}))
{% endif %}
{% endmacro %}

{% macro transform(property, source, destination) %}
{% macro transform(property, source, destination, declare_type=True) %}
{% if property.required %}
{% if property.nullable %}
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
{% else %}
{{ destination }} = {{ source }}.isoformat()
{% endif %}
{% else %}
{{ destination }}: Union[Unset, str] = UNSET
{{ destination }}{% if declare_type %}: Union[Unset, str]{% endif %} = UNSET
if not isinstance({{ source }}, Unset):
{% if property.nullable %}
{{ destination }} = {{ source }}.isoformat() if {{ source }} else None
Expand Down
6 changes: 3 additions & 3 deletions openapi_python_client/templates/property_templates/dict_property.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
{% macro construct(property, source) %}
{% macro construct(property, source, initial_value="None") %}
{% if property.required %}
{{ property.python_name }} = {{ source }}
{% else %}
{{ property.python_name }} = None
{{ property.python_name }} = {{ initial_value }}
if {{ source }} is not None:
{{ property.python_name }} = {{ source }}
{% endif %}
{% endmacro %}

{% macro transform(property, source, destination) %}
{% macro transform(property, source, destination, declare_type=True) %}
{% if property.nullable %}
{{ destination }} = {{ source }} if {{ source }} else None
{% else %}
Expand Down
12 changes: 6 additions & 6 deletions openapi_python_client/templates/property_templates/enum_property.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,27 @@
{% macro construct(property, source) %}
{% macro construct(property, source, initial_value="None") %}
{% if property.required %}
{{ property.python_name }} = {{ property.reference.class_name }}({{ source }})
{% else %}
{{ property.python_name }} = None
{{ property.python_name }} = {{ initial_value }}
if {{ source }} is not None:
{{ property.python_name }} = {{ property.reference.class_name }}({{ source }})
{% endif %}
{% endmacro %}

{% macro transform(property, source, destination) %}
{% macro transform(property, source, destination, declare_type=True) %}
{% if property.required %}
{% if property.nullable %}
{{ destination }} = {{ source }}.value if {{ source }} else None
{% else %}
{{ destination }} = {{ source }}.value
{% endif %}
{% else %}
{{ destination }}: {{ property.get_type_string() }} = UNSET
{{ destination }}{% if declare_type %}: {{ property.get_type_string() }}{% endif %} = UNSET
if not isinstance({{ source }}, Unset):
{% if property.nullable %}
{{ destination }} = {{ source }}.value if {{ source }} else None
{{ destination }} = {{ source }} if {{ source }} else None
{% else %}
{{ destination }} = {{ source }}.value
{{ destination }} = {{ source }}
{% endif %}
Comment on lines +22 to 25

@dbantydbantyNov 14, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the .values here are still necessary when converting this to JSON, unless HTTPX automatically converts Enums to their values?

@packygpackygNov 18, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a typing issue.

Mypy complained when I generated the model_with_union_property

a_property: Union[Unset, AnEnum, AnIntEnum]
try:
a_property = UNSET
if data is not None:
a_property = AnEnum(data).value
# ...

There was a type error, saying I was providing a str to a var typed as Union[Unset, AnEnum, AnIntEnum]

FWIW, json.dumpsdoes convert these enums to their primitive values (both (str, Enum) and IntEnum types). I'm assuming that's what httpx uses

@dbantydbantyNov 18, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I forgot that we inherit from (str, Enum) instead of just Enum for this exact reason. So this is great then!

{% endif %}
{% endmacro %}
6 changes: 3 additions & 3 deletions openapi_python_client/templates/property_templates/file_property.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
{% macro construct(property, source) %}
{% macro construct(property, source, initial_value=None) %}
{{ property.python_name }} = File(
payload = BytesIO({{ source }})
)
{% endmacro %}

{% macro transform(property, source, destination) %}
{% macro transform(property, source, destination, declare_type=True) %}
{% if property.required %}
{% if property.nullable %}
{{ destination }} = {{ source }}.to_tuple() if {{ source }} else None
{% else %}
{{ destination }} = {{ source }}.to_tuple()
{% endif %}
{% else %}
{{ destination }}: {{ property.get_type_string() }} = UNSET
{{ destination }}{% if declare_type %}: {{ property.get_type_string() }}{% endif %} = UNSET
if not isinstance({{ source }}, Unset):
{% if property.nullable %}
{{ destination }} = {{ source }}.to_tuple() if {{ source }} else None
Expand Down
8 changes: 4 additions & 4 deletions openapi_python_client/templates/property_templates/list_property.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
{% macro construct(property, source) %}
{% macro construct(property, source, initial_value="[]") %}
{% set inner_property = property.inner_property %}
{% if inner_property.template %}
{% set inner_source = inner_property.python_name + "_data" %}
{{ property.python_name }} = []
{{ property.python_name }} = {{ initial_value }}
{% if property.required %}
for {{ inner_source }} in ({{ source }}):
{% else %}
Expand DownExpand Up@@ -31,7 +31,7 @@ for {{ inner_source }} in {{ source }}:
{% endmacro %}


{% macro transform(property, source, destination) %}
{% macro transform(property, source, destination, declare_type=True) %}
{% set inner_property = property.inner_property %}
{% if property.required %}
{% if property.nullable %}
Expand All@@ -43,7 +43,7 @@ else:
{{ _transform(property, source, destination) }}
{% endif %}
{% else %}
{{ destination }}: Union[Unset, List[Any]] = UNSET
{{ destination }}{% if declare_type %}: Union[Unset, List[Any]]{% endif %} = UNSET
if not isinstance({{ source }}, Unset):
{% if property.nullable %}
if {{ source }} is None:
Expand Down
8 changes: 4 additions & 4 deletions openapi_python_client/templates/property_templates/model_property.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
{% macro construct(property, source) %}
{% macro construct(property, source, initial_value=None) %}
{% if property.required %}
{{ property.python_name }} = {{ property.reference.class_name }}.from_dict({{ source }})
{% else %}
{{ property.python_name }} = None
{{ property.python_name }} = {{ initial_value }}
if {{ source }} is not None:
{{ property.python_name }} = {{ property.reference.class_name }}.from_dict(cast(Dict[str, Any], {{ source }}))
{% endif %}
{% endmacro %}

{% macro transform(property, source, destination) %}
{% macro transform(property, source, destination, declare_type=True) %}
{% if property.required %}
{% if property.nullable %}
{{ destination }} = {{ source }}.to_dict() if {{ source }} else None
{% else %}
{{ destination }} = {{ source }}.to_dict()
{% endif %}
{% else %}
{{ destination }}: {{ property.get_type_string() }} = UNSET
{{ destination }}{% if declare_type %}: {{ property.get_type_string() }}{% endif %} = UNSET
if not isinstance({{ source }}, Unset):
{% if property.nullable %}
{{ destination }} = {{ source }}.to_dict() if {{ source }} else None
Expand Down
Loading