Skip to content

Better compatibility for "required" vs. "nullable" #230

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

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Cleaned up union type strings when property is not required + require…
…d but nullable properties no longer have a default value
  • Loading branch information
emann committed Nov 6, 2020
commit 9e9ebbe6adb0d12be94cef532f6cd49cf8991bda
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ def _get_kwargs(
date_prop: Union[Unset, datetime.date] = isoparse("1010-10-10").date(),
float_prop: Union[Unset, float] = 3.14,
int_prop: Union[Unset, int] = 7,
boolean_prop: Union[Unset, bool] = UNSET,
boolean_prop: Union[Unset, bool] = False,
list_prop: Union[Unset, List[AnEnum]] = UNSET,
union_prop: Union[Unset, Union[float, str]] = "not a float",
union_prop: Union[Unset, float, str] = "not a float",
enum_prop: Union[Unset, AnEnum] = UNSET,
) -> Dict[str, Any]:
url = "{}/tests/defaults".format(client.base_url)
Expand All @@ -44,7 +44,7 @@ def _get_kwargs(

json_list_prop.append(list_prop_item)

json_union_prop: Union[Unset, Union[float, str]]
json_union_prop: Union[Unset, float, str]
if isinstance(union_prop, Unset):
json_union_prop = UNSET
elif isinstance(union_prop, float):
Expand Down Expand Up @@ -114,9 +114,9 @@ def sync_detailed(
date_prop: Union[Unset, datetime.date] = isoparse("1010-10-10").date(),
float_prop: Union[Unset, float] = 3.14,
int_prop: Union[Unset, int] = 7,
boolean_prop: Union[Unset, bool] = UNSET,
boolean_prop: Union[Unset, bool] = False,
list_prop: Union[Unset, List[AnEnum]] = UNSET,
union_prop: Union[Unset, Union[float, str]] = "not a float",
union_prop: Union[Unset, float, str] = "not a float",
enum_prop: Union[Unset, AnEnum] = UNSET,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
Expand Down Expand Up @@ -149,9 +149,9 @@ def sync(
date_prop: Union[Unset, datetime.date] = isoparse("1010-10-10").date(),
float_prop: Union[Unset, float] = 3.14,
int_prop: Union[Unset, int] = 7,
boolean_prop: Union[Unset, bool] = UNSET,
boolean_prop: Union[Unset, bool] = False,
list_prop: Union[Unset, List[AnEnum]] = UNSET,
union_prop: Union[Unset, Union[float, str]] = "not a float",
union_prop: Union[Unset, float, str] = "not a float",
enum_prop: Union[Unset, AnEnum] = UNSET,
) -> Optional[Union[None, HTTPValidationError]]:
""" """
Expand Down Expand Up @@ -180,9 +180,9 @@ async def asyncio_detailed(
date_prop: Union[Unset, datetime.date] = isoparse("1010-10-10").date(),
float_prop: Union[Unset, float] = 3.14,
int_prop: Union[Unset, int] = 7,
boolean_prop: Union[Unset, bool] = UNSET,
boolean_prop: Union[Unset, bool] = False,
list_prop: Union[Unset, List[AnEnum]] = UNSET,
union_prop: Union[Unset, Union[float, str]] = "not a float",
union_prop: Union[Unset, float, str] = "not a float",
enum_prop: Union[Unset, AnEnum] = UNSET,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
Expand Down Expand Up @@ -214,9 +214,9 @@ async def asyncio(
date_prop: Union[Unset, datetime.date] = isoparse("1010-10-10").date(),
float_prop: Union[Unset, float] = 3.14,
int_prop: Union[Unset, int] = 7,
boolean_prop: Union[Unset, bool] = UNSET,
boolean_prop: Union[Unset, bool] = False,
list_prop: Union[Unset, List[AnEnum]] = UNSET,
union_prop: Union[Unset, Union[float, str]] = "not a float",
union_prop: Union[Unset, float, str] = "not a float",
enum_prop: Union[Unset, AnEnum] = UNSET,
) -> Optional[Union[None, HTTPValidationError]]:
""" """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ class AModel:
a_camel_date_time: Union[datetime.datetime, datetime.date]
a_date: datetime.date
required_not_nullable: str
some_dict: Optional[Dict[Any, Any]]
required_nullable: Optional[str]
nested_list_of_enums: Union[Unset, List[List[DifferentEnum]]] = UNSET
some_dict: Optional[Dict[Any, Any]] = None
attr_1_leading_digit: Union[Unset, str] = UNSET
required_nullable: Optional[str] = None
not_required_nullable: Union[Unset, Optional[str]] = UNSET
not_required_not_nullable: Union[Unset, str] = UNSET

Expand Down
12 changes: 6 additions & 6 deletions openapi_python_client/parser/properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,10 @@ def get_imports(self, *, prefix: str) -> Set[str]:

def to_string(self) -> str:
""" How this should be declared in a dataclass """
if self.default:
if self.default is not None:
default = self.default
elif not self.required:
default = "UNSET"
elif self.nullable:
default = "None"
else:
default = None

Expand Down Expand Up @@ -267,10 +265,12 @@ def get_type_string(self, no_optional: bool = False) -> str:
inner_types = [p.get_type_string(no_optional=True) for p in self.inner_properties]
inner_prop_string = ", ".join(inner_types)
type_string = f"Union[{inner_prop_string}]"
if not no_optional and self.nullable:
if no_optional:
return type_string
if not self.required:
type_string = f"Union[Unset, {inner_prop_string}]"
if self.nullable:
type_string = f"Optional[{type_string}]"
if not no_unset and not self.required:
type_string = f"Union[Unset, {type_string}]"
return type_string

def get_imports(self, *, prefix: str) -> Set[str]:
Expand Down
7 changes: 7 additions & 0 deletions openapi_python_client/templates/model.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@ from ..types import UNSET, Unset
class {{ model.reference.class_name }}:
""" {{ model.description }} """
{% for property in model.required_properties + model.optional_properties %}
{% if property.default is none and property.required %}
{{ property.to_string() }}
{% endif %}
{% endfor %}
{% for property in model.required_properties + model.optional_properties %}
{% if property.default is not none or not property.required %}
{{ property.to_string() }}
{% endif %}
{% endfor %}

def to_dict(self) -> Dict[str, Any]:
Expand Down
22 changes: 19 additions & 3 deletions tests/test_openapi_parser/test_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,15 @@ def test_get_type_string(self):

p.nullable = True
assert p.get_type_string() == f"Optional[{base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

p.required = False
assert p.get_type_string() == f"Union[Unset, Optional[{base_type_string}]]"
assert p.get_type_string(no_optional=True) == base_type_string

p.nullable = False
assert p.get_type_string() == f"Union[Unset, {base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

def test_to_string(self, mocker):
from openapi_python_client.parser.properties import Property
Expand All @@ -51,7 +54,7 @@ def test_to_string(self, mocker):

p.required = True
p.nullable = True
assert p.to_string() == f"{name}: {get_type_string()} = None"
assert p.to_string() == f"{name}: {get_type_string()}"

p.default = "TEST"
assert p.to_string() == f"{name}: {get_type_string()} = TEST"
Expand Down Expand Up @@ -270,12 +273,15 @@ def test_get_type_string(self, mocker):

p.nullable = True
assert p.get_type_string() == f"Optional[{base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

p.required = False
assert p.get_type_string() == f"Union[Unset, Optional[{base_type_string}]]"
assert p.get_type_string(no_optional=True) == base_type_string

p.nullable = False
assert p.get_type_string() == f"Union[Unset, {base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

def test_get_type_imports(self, mocker):
from openapi_python_client.parser.properties import ListProperty
Expand Down Expand Up @@ -338,12 +344,16 @@ def test_get_type_string(self, mocker):

p.nullable = True
assert p.get_type_string() == f"Optional[{base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

base_type_string_with_unset = f"Union[Unset, inner_type_string_1, inner_type_string_2]"
p.required = False
assert p.get_type_string() == f"Union[Unset, Optional[{base_type_string}]]"
assert p.get_type_string() == f"Optional[{base_type_string_with_unset}]"
assert p.get_type_string(no_optional=True) == base_type_string

p.nullable = False
assert p.get_type_string() == f"Union[Unset, {base_type_string}]"
assert p.get_type_string() == base_type_string_with_unset
assert p.get_type_string(no_optional=True) == base_type_string

def test_get_type_imports(self, mocker):
from openapi_python_client.parser.properties import UnionProperty
Expand Down Expand Up @@ -483,12 +493,15 @@ def test_get_type_string(self, mocker):

p.nullable = True
assert p.get_type_string() == f"Optional[{base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

p.required = False
assert p.get_type_string() == f"Union[Unset, Optional[{base_type_string}]]"
assert p.get_type_string(no_optional=True) == base_type_string

p.nullable = False
assert p.get_type_string() == f"Union[Unset, {base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

properties._existing_enums = {}

Expand Down Expand Up @@ -616,12 +629,15 @@ def test_get_type_string(self, mocker):

p.nullable = True
assert p.get_type_string() == f"Optional[{base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

p.required = False
assert p.get_type_string() == f"Union[Unset, Optional[{base_type_string}]]"
assert p.get_type_string(no_optional=True) == base_type_string

p.nullable = False
assert p.get_type_string() == f"Union[Unset, {base_type_string}]"
assert p.get_type_string(no_optional=True) == base_type_string

def test_get_imports(self, mocker):
fake_reference = mocker.MagicMock(class_name="MyRefClass", module_name="my_test_enum")
Expand Down