Skip to content
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

[Python-experimental] types now classes, adds additionalProperties handling #4154

Merged
merged 13 commits into from
Oct 28, 2019

Conversation

spacether
Copy link
Contributor

@spacether spacether commented Oct 14, 2019

This PR changes the type descriptions in model classes from strings to real python classes
so what was 'str' now becomes str.
This PR also adds the feature where additional properties defined in models can be strictly typed to one type like str, or can allow any type, like (str, float, int, date, datetime, list, dict).
Note: I needed to turn on the additionalproperties handling so our tests would pass for string_boolean_map.py
This PR also turns on automatic type checking when instantiating a model on the client side or ingesting the model from the server.

  • If the type is incorrect and _check_type=True, an ApiTypeError exception will be raised
  • That type checking can be turned off by setting _check_type=False.

This PR is the final piece implementing the solution in #2049

Details

  • Adds support for additionalProperties to the Python generator
  • Fixes inheritance in python when AllOf is used
  • Adds the ability to access and assign values to a model instance using brackets. model_instance['var_name'] = var_val
  • Simplifies model instance variable storage by storing all values in a data dict. This will make it easier to send Nullable value=None items, because items in the data store are correct
  • Switches parameter and discriminator typing to use actual python classes
  • Adds type checking with TypeError/ValueError/KeyError exceptions which tell where in the data the problem is (data is in the exception string and exception.path_to_item)
  • Model instances can be instantiated with type checking with the _check_type variable
  • Endpoint hits now check type before sending out data with the _check_input_type variable
  • Deserialization automatically checks type on all received data
  • If an optional value is accessed like model.optional_var and optional_var was not set when creating the instance we will now raise an ApiKeyError

Issues that will be closed if this is merged:

Inheritance not working: #623
When model only has allOf, it is blank: #453
Fixes additionalproperties: #2028
$ref references to non-object types not working correctly: #1991

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before .
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10) @Jyhess (2019/01)

@spacether spacether force-pushed the issue_1991_uses_class_types branch from 9f186b1 to ffe4b7b Compare October 14, 2019 19:28
@spacether spacether changed the title Python-experimental: types now classes, adds additionalProperties handling [Python-experimental] types now classes, adds additionalProperties handling Oct 14, 2019
…ties handling

Adds model_utils update, updates model.mustache

Updates api.mustache and uses model_to_dict in model_normal.mustache

Updates requirements.mustache for PythonClientExperimental

Passes through check_type when deserializing models

Converts types from strings to classes in PythonClientExperimentalCodegen.java and PythonTest.java

Creates PythonClientExperimentalTest.java

Updates toInstantiationType to use ModelUtils.xxx

Corrects docstring descriptions of response_type

Updates python-experimental typing, partially fixes deserialization tests

Adds fixes for some of the deserialization tests

Fixes deserialization tests

Switches model teplates to use allVars so allof props will be included

Fixes tests.test_enum_arrays

Fixes test_to_str

Adds additional_properties_type, fixes teast_todict in test_map_test.py

Correctly check the type of _request_timeout values

Fixes test_upload_file test

Turns off coercion when instantiating model types with client data

Updates file handling to input and output an open file object

Fixes linting errors

Adds fixes for python2 tests, linting fixes

Adds additionalproperties to docs + tests

Regenerates python-experimatal client
@spacether spacether force-pushed the issue_1991_uses_class_types branch from ba94b27 to 2ee49a1 Compare October 16, 2019 18:23
@spacether
Copy link
Contributor Author

@wing328 all tests pass. This PR is ready to be reviewed

@spacether
Copy link
Contributor Author

spacether commented Oct 22, 2019

@wing328 all better, file uploading now works correctly. I added a test for uploading multiple files at test_pet_api.py:test_upload_file.
All tests pass, ready for another review.

@wing328 wing328 merged commit 73c55c1 into OpenAPITools:master Oct 28, 2019
@spacether
Copy link
Contributor Author

@mverderese @ivan-gomes this feature can now be used in the Python-experimental generator

@wing328
Copy link
Member

wing328 commented Oct 31, 2019

@spacether thanks for the PR, which has been included in v4.2.0 release: https://twitter.com/oas_generator/status/1189824932345069569

@spacether spacether deleted the issue_1991_uses_class_types branch January 3, 2020 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants