Skip to content

[python-nextgen] optionally support float strict type #14618

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
merged 2 commits into from
Feb 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions bin/configs/python-nextgen-aiohttp.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ templateDir: modules/openapi-generator/src/main/resources/python-nextgen
library: asyncio
additionalProperties:
packageName: petstore_api
floatStrictType: false
1 change: 1 addition & 0 deletions docs/generators/python-nextgen.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
| Option | Description | Values | Default |
| ------ | ----------- | ------ | ------- |
|disallowAdditionalPropertiesIfNotPresent|If false, the 'additionalProperties' implementation (set to true by default) is compliant with the OAS and JSON schema specifications. If true (default), keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default.|<dl><dt>**false**</dt><dd>The 'additionalProperties' implementation is compliant with the OAS and JSON schema specifications.</dd><dt>**true**</dt><dd>Keep the old (incorrect) behaviour that 'additionalProperties' is set to false by default.</dd></dl>|true|
|floatStrictType|Use strict type for float, i.e. StrictFloat or confloat(strict=true, ...)| |true|
|generateSourceCodeOnly|Specifies that only a library source code is to be generated.| |false|
|hideGenerationTimestamp|Hides the generation timestamp when files are generated.| |true|
|library|library template (sub-template) to use: asyncio, tornado (deprecated), urllib3| |urllib3|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,14 @@ public class PythonNextgenClientCodegen extends AbstractPythonCodegen implements
public static final String PACKAGE_URL = "packageUrl";
public static final String DEFAULT_LIBRARY = "urllib3";
public static final String RECURSION_LIMIT = "recursionLimit";
public static final String PYTHON_ATTR_NONE_IF_UNSET = "pythonAttrNoneIfUnset";
public static final String FLOAT_STRICT_TYPE = "floatStrictType";

protected String packageUrl;
protected String apiDocPath = "docs" + File.separator;
protected String modelDocPath = "docs" + File.separator;
protected boolean hasModelsToImport = Boolean.FALSE;
protected boolean useOneOfDiscriminatorLookup = false; // use oneOf discriminator's mapping for model lookup
protected boolean floatStrictType = true;

protected Map<Character, String> regexModifiers;

Expand Down Expand Up @@ -164,6 +165,8 @@ public PythonNextgenClientCodegen() {
cliOptions.add(new CliOption(CodegenConstants.SOURCECODEONLY_GENERATION, CodegenConstants.SOURCECODEONLY_GENERATION_DESC)
.defaultValue(Boolean.FALSE.toString()));
cliOptions.add(new CliOption(RECURSION_LIMIT, "Set the recursion limit. If not set, use the system default value."));
cliOptions.add(new CliOption(FLOAT_STRICT_TYPE, "Use strict type for float, i.e. StrictFloat or confloat(strict=true, ...)")
.defaultValue(Boolean.TRUE.toString()));

supportedLibraries.put("urllib3", "urllib3-based client");
supportedLibraries.put("asyncio", "asyncio-based client");
Expand Down Expand Up @@ -259,6 +262,10 @@ public void processOpts() {
additionalProperties.put(CodegenConstants.USE_ONEOF_DISCRIMINATOR_LOOKUP, useOneOfDiscriminatorLookup);
}

if (additionalProperties.containsKey(FLOAT_STRICT_TYPE)) {
setFloatStrictType(convertPropertyToBooleanAndWriteBack(FLOAT_STRICT_TYPE));
}

String modelPath = packagePath() + File.separatorChar + modelPackage.replace('.', File.separatorChar);
String apiPath = packagePath() + File.separatorChar + apiPackage.replace('.', File.separatorChar);

Expand Down Expand Up @@ -424,7 +431,6 @@ private String getPydanticType(CodegenParameter cp,
if (cp.hasValidation) {
List<String> fieldCustomization = new ArrayList<>();
// e.g. confloat(ge=10, le=100, strict=True)
fieldCustomization.add("strict=True");
if (cp.getMaximum() != null) {
if (cp.getExclusiveMaximum()) {
fieldCustomization.add("gt=" + cp.getMaximum());
Expand All @@ -443,12 +449,20 @@ private String getPydanticType(CodegenParameter cp,
fieldCustomization.add("multiple_of=" + cp.getMultipleOf());
}

if (floatStrictType) {
Copy link
Contributor

@spacether spacether Feb 4, 2023

Choose a reason for hiding this comment

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

This code path is used for json schema number (float + int) and float only processing. Number should allow both in.

How about:

  • not adding the strict info when it is number with no format
  • adding it when it is float only
  • adding tests of both of those cases (number vs float only) with and without strict on

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an option so users can choose whether they want float in strict type or not.

Copy link
Contributor

@spacether spacether Feb 7, 2023

Choose a reason for hiding this comment

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

Right but that option should only apply to floats right? The way it is now looks like you are applying float strict when the spec has type: number which should allow in float and int. Did I misunderstand something?

Copy link
Member Author

Choose a reason for hiding this comment

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

-    number: confloat(strict=True, le=543.2, ge=32.1) = ...
-   float: Optional[confloat(strict=True, le=987.6, ge=54.3)] = None
-    double: Optional[confloat(strict=True, le=123.4, ge=67.8)] = None
+    number: confloat(le=543.2, ge=32.1) = ...
+    float: Optional[confloat(le=987.6, ge=54.3)] = None
+    double: Optional[confloat(le=123.4, ge=67.8)] = None

As illustrated in the sample diff as part of this change, the option (when set to false) simply removes strict=True.

Copy link
Contributor

@spacether spacether Feb 8, 2023

Choose a reason for hiding this comment

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

Why are you applying strict float to type number with no format? Won't it throw a validation error if an integer input is given to it? Integer input is valid for that definition per
https://json-schema.org/understanding-json-schema/reference/numeric.html#number

Copy link
Member Author

Choose a reason for hiding this comment

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

Why are you applying strict float to type number with no format?

It's a user decision - they can switch on or off strict mode via an option.

You can find more about why some users prefer strict mode in https://docs.pydantic.dev/blog/pydantic-v2/#strict-mode.

fieldCustomization.add("strict=True");
}

pydanticImports.add("confloat");
return String.format(Locale.ROOT, "%s(%s)", "confloat",
StringUtils.join(fieldCustomization, ", "));
} else {
pydanticImports.add("StrictFloat");
return "StrictFloat";
if (floatStrictType) {
pydanticImports.add("StrictFloat");
return "StrictFloat";
} else {
return "float";
}
}
} else if (cp.isInteger || cp.isLong || cp.isShort || cp.isUnboundedInteger) {
if (cp.hasValidation) {
Expand Down Expand Up @@ -645,7 +659,6 @@ private String getPydanticType(CodegenProperty cp,
if (cp.hasValidation) {
List<String> fieldCustomization = new ArrayList<>();
// e.g. confloat(ge=10, le=100, strict=True)
fieldCustomization.add("strict=True");
if (cp.getMaximum() != null) {
if (cp.getExclusiveMaximum()) {
fieldCustomization.add("lt=" + cp.getMaximum());
Expand All @@ -664,12 +677,20 @@ private String getPydanticType(CodegenProperty cp,
fieldCustomization.add("multiple_of=" + cp.getMultipleOf());
}

if (floatStrictType) {
fieldCustomization.add("strict=True");
}

pydanticImports.add("confloat");
return String.format(Locale.ROOT, "%s(%s)", "confloat",
StringUtils.join(fieldCustomization, ", "));
} else {
pydanticImports.add("StrictFloat");
return "StrictFloat";
if (floatStrictType) {
pydanticImports.add("StrictFloat");
return "StrictFloat";
} else {
return "float";
}
}
} else if (cp.isInteger || cp.isLong || cp.isShort || cp.isUnboundedInteger) {
if (cp.hasValidation) {
Expand Down Expand Up @@ -1316,4 +1337,8 @@ public String escapeReservedWord(String name) {
}
return "var_" + name;
}

public void setFloatStrictType(boolean floatStrictType) {
this.floatStrictType = floatStrictType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1521,7 +1521,6 @@ components:
type: object
required:
- number
- byte
- date
- password
properties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Name | Type | Description | Notes
**double** | **float** | | [optional]
**decimal** | **decimal.Decimal** | | [optional]
**string** | **str** | | [optional]
**byte** | **str** | |
**byte** | **str** | | [optional]
**binary** | **str** | | [optional]
**var_date** | **date** | |
**date_time** | **datetime** | | [optional]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

from datetime import date, datetime

from pydantic import Field, StrictBool, StrictFloat, StrictInt, StrictStr, confloat, conint, constr, validator
from pydantic import Field, StrictBool, StrictInt, StrictStr, confloat, conint, constr, validator

from typing import Dict, List, Optional

Expand Down Expand Up @@ -641,7 +641,7 @@ def fake_outer_composite_serialize_with_http_info(self, outer_composite : Annota
_request_auth=_params.get('_request_auth'))

@validate_arguments
def fake_outer_number_serialize(self, body : Annotated[Optional[StrictFloat], Field(description="Input number as post body")] = None, **kwargs) -> float: # noqa: E501
def fake_outer_number_serialize(self, body : Annotated[Optional[float], Field(description="Input number as post body")] = None, **kwargs) -> float: # noqa: E501
"""fake_outer_number_serialize # noqa: E501

Test serialization of outer number types # noqa: E501
Expand Down Expand Up @@ -672,7 +672,7 @@ def fake_outer_number_serialize(self, body : Annotated[Optional[StrictFloat], Fi
return self.fake_outer_number_serialize_with_http_info(body, **kwargs) # noqa: E501

@validate_arguments
def fake_outer_number_serialize_with_http_info(self, body : Annotated[Optional[StrictFloat], Field(description="Input number as post body")] = None, **kwargs): # noqa: E501
def fake_outer_number_serialize_with_http_info(self, body : Annotated[Optional[float], Field(description="Input number as post body")] = None, **kwargs): # noqa: E501
"""fake_outer_number_serialize # noqa: E501

Test serialization of outer number types # noqa: E501
Expand Down Expand Up @@ -1678,7 +1678,7 @@ def test_client_model_with_http_info(self, client : Annotated[Client, Field(...,
_request_auth=_params.get('_request_auth'))

@validate_arguments
def test_endpoint_parameters(self, number : Annotated[confloat(strict=True, ge=543.2, le=32.1), Field(..., description="None")], double : Annotated[confloat(strict=True, ge=123.4, le=67.8), Field(..., description="None")], pattern_without_delimiter : Annotated[constr(strict=True), Field(..., description="None")], byte : Annotated[StrictStr, Field(..., description="None")], integer : Annotated[Optional[conint(strict=True, le=100, ge=10)], Field(description="None")] = None, int32 : Annotated[Optional[conint(strict=True, le=200, ge=20)], Field(description="None")] = None, int64 : Annotated[Optional[StrictInt], Field(description="None")] = None, float : Annotated[Optional[confloat(strict=True, ge=987.6)], Field(description="None")] = None, string : Annotated[Optional[constr(strict=True)], Field(description="None")] = None, binary : Annotated[Optional[StrictStr], Field(description="None")] = None, var_date : Annotated[Optional[date], Field(description="None")] = None, date_time : Annotated[Optional[datetime], Field(description="None")] = None, password : Annotated[Optional[constr(strict=True, max_length=64, min_length=10)], Field(description="None")] = None, param_callback : Annotated[Optional[StrictStr], Field(description="None")] = None, **kwargs) -> None: # noqa: E501
def test_endpoint_parameters(self, number : Annotated[confloat(ge=543.2, le=32.1), Field(..., description="None")], double : Annotated[confloat(ge=123.4, le=67.8), Field(..., description="None")], pattern_without_delimiter : Annotated[constr(strict=True), Field(..., description="None")], byte : Annotated[StrictStr, Field(..., description="None")], integer : Annotated[Optional[conint(strict=True, le=100, ge=10)], Field(description="None")] = None, int32 : Annotated[Optional[conint(strict=True, le=200, ge=20)], Field(description="None")] = None, int64 : Annotated[Optional[StrictInt], Field(description="None")] = None, float : Annotated[Optional[confloat(ge=987.6)], Field(description="None")] = None, string : Annotated[Optional[constr(strict=True)], Field(description="None")] = None, binary : Annotated[Optional[StrictStr], Field(description="None")] = None, var_date : Annotated[Optional[date], Field(description="None")] = None, date_time : Annotated[Optional[datetime], Field(description="None")] = None, password : Annotated[Optional[constr(strict=True, max_length=64, min_length=10)], Field(description="None")] = None, param_callback : Annotated[Optional[StrictStr], Field(description="None")] = None, **kwargs) -> None: # noqa: E501
"""Fake endpoint for testing various parameters 假端點 偽のエンドポイント 가짜 엔드 포인트 # noqa: E501

Fake endpoint for testing various parameters 假端點 偽のエンドポイント 가짜 엔드 포인트 # noqa: E501
Expand Down Expand Up @@ -1735,7 +1735,7 @@ def test_endpoint_parameters(self, number : Annotated[confloat(strict=True, ge=5
return self.test_endpoint_parameters_with_http_info(number, double, pattern_without_delimiter, byte, integer, int32, int64, float, string, binary, var_date, date_time, password, param_callback, **kwargs) # noqa: E501

@validate_arguments
def test_endpoint_parameters_with_http_info(self, number : Annotated[confloat(strict=True, ge=543.2, le=32.1), Field(..., description="None")], double : Annotated[confloat(strict=True, ge=123.4, le=67.8), Field(..., description="None")], pattern_without_delimiter : Annotated[constr(strict=True), Field(..., description="None")], byte : Annotated[StrictStr, Field(..., description="None")], integer : Annotated[Optional[conint(strict=True, le=100, ge=10)], Field(description="None")] = None, int32 : Annotated[Optional[conint(strict=True, le=200, ge=20)], Field(description="None")] = None, int64 : Annotated[Optional[StrictInt], Field(description="None")] = None, float : Annotated[Optional[confloat(strict=True, ge=987.6)], Field(description="None")] = None, string : Annotated[Optional[constr(strict=True)], Field(description="None")] = None, binary : Annotated[Optional[StrictStr], Field(description="None")] = None, var_date : Annotated[Optional[date], Field(description="None")] = None, date_time : Annotated[Optional[datetime], Field(description="None")] = None, password : Annotated[Optional[constr(strict=True, max_length=64, min_length=10)], Field(description="None")] = None, param_callback : Annotated[Optional[StrictStr], Field(description="None")] = None, **kwargs): # noqa: E501
def test_endpoint_parameters_with_http_info(self, number : Annotated[confloat(ge=543.2, le=32.1), Field(..., description="None")], double : Annotated[confloat(ge=123.4, le=67.8), Field(..., description="None")], pattern_without_delimiter : Annotated[constr(strict=True), Field(..., description="None")], byte : Annotated[StrictStr, Field(..., description="None")], integer : Annotated[Optional[conint(strict=True, le=100, ge=10)], Field(description="None")] = None, int32 : Annotated[Optional[conint(strict=True, le=200, ge=20)], Field(description="None")] = None, int64 : Annotated[Optional[StrictInt], Field(description="None")] = None, float : Annotated[Optional[confloat(ge=987.6)], Field(description="None")] = None, string : Annotated[Optional[constr(strict=True)], Field(description="None")] = None, binary : Annotated[Optional[StrictStr], Field(description="None")] = None, var_date : Annotated[Optional[date], Field(description="None")] = None, date_time : Annotated[Optional[datetime], Field(description="None")] = None, password : Annotated[Optional[constr(strict=True, max_length=64, min_length=10)], Field(description="None")] = None, param_callback : Annotated[Optional[StrictStr], Field(description="None")] = None, **kwargs): # noqa: E501
"""Fake endpoint for testing various parameters 假端點 偽のエンドポイント 가짜 엔드 포인트 # noqa: E501

Fake endpoint for testing various parameters 假端點 偽のエンドポイント 가짜 엔드 포인트 # noqa: E501
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@


from typing import List, Optional
from pydantic import BaseModel, Field, StrictFloat
from pydantic import BaseModel, Field

class ArrayOfArrayOfNumberOnly(BaseModel):
"""NOTE: This class is auto generated by OpenAPI Generator.
Ref: https://openapi-generator.tech

Do not edit the class manually.
"""
array_array_number: Optional[List[List[StrictFloat]]] = Field(None, alias="ArrayArrayNumber")
array_array_number: Optional[List[List[float]]] = Field(None, alias="ArrayArrayNumber")
__properties = ["ArrayArrayNumber"]

class Config:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@


from typing import List, Optional
from pydantic import BaseModel, Field, StrictFloat
from pydantic import BaseModel, Field

class ArrayOfNumberOnly(BaseModel):
"""NOTE: This class is auto generated by OpenAPI Generator.
Ref: https://openapi-generator.tech

Do not edit the class manually.
"""
array_number: Optional[List[StrictFloat]] = Field(None, alias="ArrayNumber")
array_number: Optional[List[float]] = Field(None, alias="ArrayNumber")
__properties = ["ArrayNumber"]

class Config:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@


from typing import Optional
from pydantic import BaseModel, Field, StrictFloat, StrictInt, StrictStr, validator
from pydantic import BaseModel, Field, StrictInt, StrictStr, validator
from petstore_api.models.outer_enum import OuterEnum
from petstore_api.models.outer_enum_default_value import OuterEnumDefaultValue
from petstore_api.models.outer_enum_integer import OuterEnumInteger
Expand All @@ -33,7 +33,7 @@ class EnumTest(BaseModel):
enum_string: Optional[StrictStr] = None
enum_string_required: StrictStr = ...
enum_integer: Optional[StrictInt] = None
enum_number: Optional[StrictFloat] = None
enum_number: Optional[float] = None
outer_enum: Optional[OuterEnum] = Field(None, alias="outerEnum")
outer_enum_integer: Optional[OuterEnumInteger] = Field(None, alias="outerEnumInteger")
outer_enum_default_value: Optional[OuterEnumDefaultValue] = Field(None, alias="outerEnumDefaultValue")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ class FormatTest(BaseModel):
integer: Optional[conint(strict=True, le=100, ge=10)] = None
int32: Optional[conint(strict=True, le=200, ge=20)] = None
int64: Optional[StrictInt] = None
number: confloat(strict=True, le=543.2, ge=32.1) = ...
float: Optional[confloat(strict=True, le=987.6, ge=54.3)] = None
double: Optional[confloat(strict=True, le=123.4, ge=67.8)] = None
number: confloat(le=543.2, ge=32.1) = ...
float: Optional[confloat(le=987.6, ge=54.3)] = None
double: Optional[confloat(le=123.4, ge=67.8)] = None
decimal: Optional[condecimal()] = None
string: Optional[constr(strict=True)] = None
byte: StrictBytes = ...
byte: Optional[StrictBytes] = None
binary: Optional[StrictBytes] = None
var_date: date = Field(..., alias="date")
date_time: Optional[datetime] = Field(None, alias="dateTime")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from datetime import date, datetime
from typing import Any, Dict, List, Optional
from pydantic import BaseModel, StrictBool, StrictFloat, StrictInt, StrictStr
from pydantic import BaseModel, StrictBool, StrictInt, StrictStr

class NullableClass(BaseModel):
"""NOTE: This class is auto generated by OpenAPI Generator.
Expand All @@ -28,7 +28,7 @@ class NullableClass(BaseModel):
"""
required_integer_prop: Optional[StrictInt] = ...
integer_prop: Optional[StrictInt] = None
number_prop: Optional[StrictFloat] = None
number_prop: Optional[float] = None
boolean_prop: Optional[StrictBool] = None
string_prop: Optional[StrictStr] = None
date_prop: Optional[date] = None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@


from typing import Optional
from pydantic import BaseModel, Field, StrictFloat
from pydantic import BaseModel, Field

class NumberOnly(BaseModel):
"""NOTE: This class is auto generated by OpenAPI Generator.
Ref: https://openapi-generator.tech

Do not edit the class manually.
"""
just_number: Optional[StrictFloat] = Field(None, alias="JustNumber")
just_number: Optional[float] = Field(None, alias="JustNumber")
__properties = ["JustNumber"]

class Config:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@


from typing import List, Optional
from pydantic import BaseModel, Field, StrictFloat, StrictStr
from pydantic import BaseModel, Field, StrictStr
from petstore_api.models.deprecated_object import DeprecatedObject

class ObjectWithDeprecatedFields(BaseModel):
Expand All @@ -28,7 +28,7 @@ class ObjectWithDeprecatedFields(BaseModel):
Do not edit the class manually.
"""
uuid: Optional[StrictStr] = None
id: Optional[StrictFloat] = None
id: Optional[float] = None
deprecated_ref: Optional[DeprecatedObject] = Field(None, alias="deprecatedRef")
bars: Optional[List[StrictStr]] = None
__properties = ["uuid", "id", "deprecatedRef", "bars"]
Expand Down
Loading