-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
python: several typing and style improvements #16378
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,14 +23,14 @@ from {{packageName}}.exceptions import ( # noqa: F401 | |
|
|
||
|
|
||
| {{#operations}} | ||
| class {{classname}}(object): | ||
| class {{classname}}: | ||
| """NOTE: This class is auto generated by OpenAPI Generator | ||
| Ref: https://openapi-generator.tech | ||
|
|
||
| Do not edit the class manually. | ||
| """ | ||
|
|
||
| def __init__(self, api_client=None): | ||
| def __init__(self, api_client=None) -> None: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The return type of |
||
| if api_client is None: | ||
| api_client = ApiClient.get_default() | ||
| self.api_client = api_client | ||
|
|
@@ -65,18 +65,19 @@ class {{classname}}(object): | |
| {{/allParams}} | ||
| :param async_req: Whether to execute the request asynchronously. | ||
| :type async_req: bool, optional | ||
| :param _request_timeout: timeout setting for this request. If one | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This alignment avoids line-length issues (using |
||
| number provided, it will be total request | ||
| timeout. It can also be a pair (tuple) of | ||
| (connection, read) timeouts. | ||
| :param _request_timeout: timeout setting for this request. | ||
| If one number provided, it will be total request | ||
| timeout. It can also be a pair (tuple) of | ||
| (connection, read) timeouts. | ||
| :return: Returns the result object. | ||
| If the method is called asynchronously, | ||
| returns the request thread. | ||
| :rtype: {{returnType}}{{^returnType}}None{{/returnType}} | ||
| """ | ||
| kwargs['_return_http_data_only'] = True | ||
| if '_preload_content' in kwargs: | ||
| raise ValueError("Error! Please call the {{operationId}}_with_http_info method with `_preload_content` instead and obtain raw data from ApiResponse.raw_data") | ||
| message = "Error! Please call the {{operationId}}_with_http_info method with `_preload_content` instead and obtain raw data from ApiResponse.raw_data" # noqa: E501 | ||
| raise ValueError(message) | ||
|
Comment on lines
79
to
80
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some formatters (e.g. raise ValueError("long string") # noqa: E501to: raise ValueError(
"long string"
) # noqa: E501thereby moving the |
||
| {{#asyncio}} | ||
| if async_req is not None: | ||
| kwargs['async_req'] = async_req | ||
|
|
@@ -103,7 +104,7 @@ class {{classname}}(object): | |
| :param async_req: Whether to execute the request asynchronously. | ||
| :type async_req: bool, optional | ||
| :param _preload_content: if False, the ApiResponse.data will | ||
| be set to none and raw_data will store the | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| be set to none and raw_data will store the | ||
| HTTP response body without reading/decoding. | ||
| Default is True. | ||
| :type _preload_content: bool, optional | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ from {{packageName}} import rest | |
| from {{packageName}}.exceptions import ApiValueError, ApiException | ||
|
|
||
|
|
||
| class ApiClient(object): | ||
| class ApiClient: | ||
| """Generic API client for OpenAPI client library builds. | ||
|
|
||
| OpenAPI generic API client. This client handles the client- | ||
|
|
@@ -56,7 +56,7 @@ class ApiClient(object): | |
| _pool = None | ||
|
|
||
| def __init__(self, configuration=None, header_name=None, header_value=None, | ||
| cookie=None, pool_threads=1): | ||
| cookie=None, pool_threads=1) -> None: | ||
| # use default configuration if none is provided | ||
| if configuration is None: | ||
| configuration = Configuration.get_default() | ||
|
|
@@ -345,7 +345,7 @@ class ApiClient(object): | |
| if data is None: | ||
| return None | ||
|
|
||
| if type(klass) == str: | ||
| if isinstance(klass, str): | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was just bad Python (and |
||
| if klass.startswith('List['): | ||
| sub_kls = re.match(r'List\[(.*)]', klass).group(1) | ||
| return [self.__deserialize(sub_data, sub_kls) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,22 +4,20 @@ | |
|
|
||
| import unittest | ||
|
|
||
| import {{packageName}} | ||
| from {{apiPackage}}.{{classFilename}} import {{classname}} # noqa: E501 | ||
| from {{packageName}}.rest import ApiException | ||
|
Comment on lines
-7
to
-9
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not clear what the right behavior is for a test case that is meant to be edited by hand. On the one hand, adding imports for likely usages is helpful; on the other hand, it's not great to generate code that doesn't pass the linter because it includes unnecessary imports.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about commenting out these imports instead so that users can easily uncomment these imports if needed? |
||
|
|
||
|
|
||
| class {{#operations}}Test{{classname}}(unittest.TestCase): | ||
| """{{classname}} unit test stubs""" | ||
|
|
||
| def setUp(self): | ||
| self.api = {{apiPackage}}.{{classFilename}}.{{classname}}() # noqa: E501 | ||
| def setUp(self) -> None: | ||
| self.api = {{classname}}() # noqa: E501 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The import block was importing both the package and the class; we only need one of these. It's more common in Python to use |
||
|
|
||
| def tearDown(self): | ||
| def tearDown(self) -> None: | ||
| pass | ||
|
|
||
| {{#operation}} | ||
| def test_{{operationId}}(self): | ||
| def test_{{operationId}}(self) -> None: | ||
| """Test case for {{{operationId}}} | ||
|
|
||
| {{#summary}} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use of
(object)as a base class has not been recommended for a long time.