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] Adding type-hinting for API responses #841

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Dringho
Copy link

@Dringho Dringho commented Jan 11, 2021

Tested in Pycharm and vscode

image

@Dringho Dringho changed the title [Python] Adding type-hinting for API [Python] Adding type-hinting for API responses Jan 11, 2021
@Dringho Dringho closed this Jan 14, 2021
@HugoMario
Copy link
Contributor

Hey @Dringho , why you closed this? Need any help?

@Dringho Dringho reopened this Jan 15, 2021
@Dringho
Copy link
Author

Dringho commented Jan 15, 2021

@HugoMario I was trying to improve the solution but I couldn't without delving into the Java, which was a mess to understand.

My current problem it's that on the generated inline_response_200 response, if you have a nested structure, it references to answers like inline_response_200_message, but there was no way to add the import for it, so the type hinting would be enabled at the nested level.

I was working on the python/model.mustache file adding imports like this

# Importing related models
{{#models}}{{#model}}{{#vars}}{{#complexType}}from .I_CANT_GET_THIS_RIGHT import {{complexType}}
{{/complexType}}{{/vars}}{{/model}}{{/models}}

This is because of the SnakeCase vs CamelCase difference between the filename and the class name. Currently there is no way to get the snake_case name of the complex type. This is the JSON that was supplied to mustache

"vars" : [ {
     vendorExtensions" : {
        "x-is-container" : true,
        "x-is-list-container" : true
      },
      "baseName" : "message",
      "complexType" : "InlineResponse20010Message",
      "getter" : "getMessage",
      "setter" : "setMessage",
      "datatype" : "list[InlineResponse20010Message]",
      "datatypeWithEnum" : "list[InlineResponse20010Message]",
      "name" : "message",
      "defaultValueWithParam" : " = data.message;",
      "baseType" : "list",
      "containerType" : "array",
      "jsonSchema" : "{\n  \"type\" : \"array\",\n  \"items\" : {\n    \"$ref\" : \"#/components/schemas/inline_response_200_10_message\"\n  }\n}",
      "exclusiveMinimum" : false,
      "exclusiveMaximum" : false,
      "required" : false,
      "secondaryParam" : false,
      "nullable" : false,
      "items" : {
        "vendorExtensions" : {
          "x-is-object" : true,
          "x-is-not-container" : true
        },
        "baseName" : "message",
        "complexType" : "InlineResponse20010Message",
        "getter" : "getMessage",
        "setter" : "setMessage",
        "datatype" : "InlineResponse20010Message",
        "datatypeWithEnum" : "InlineResponse20010Message",
        "name" : "message",
        "defaultValueWithParam" : " = data.message;",
        "baseType" : "InlineResponse20010Message",
        "jsonSchema" : "{\n  \"$ref\" : \"#/components/schemas/inline_response_200_10_message\"\n}",
        "exclusiveMinimum" : false,
        "exclusiveMaximum" : false,
        "required" : false,
        "secondaryParam" : false,
        "nullable" : false,
        "nameInCamelCase" : "Message",
        "iexclusiveMaximum" : false,
        "moreNonReadOnly" : false,
        "isPrimitiveType" : false,
        "isDateTime" : false,
        "isUuid" : false,
        "isDefault" : false,
        "isMapContainer" : false,
        "isListContainer" : false,
        "isMultipart" : false,
        "isResponseBinary" : false,
        "isResponseFile" : false,
        "isBinary" : false,
        "isFile" : false,
        "isEnum" : false,
        "isArrayModel" : false,
        "isAlias" : false,
        "isObject" : true,
        "hasInnerObject" : false,
        "isContainer" : false,
        "isNotContainer" : true,
        "isReadOnly" : false,
        "isCollectionFormatMulti" : false,
        "hasMore" : false,
        "isInteger" : false,
        "isNumber" : false,
        "hasHeaders" : false,
        "isString" : false,
        "isNumeric" : false,
        "isLong" : false,
        "isFloat" : false,
        "isDouble" : false,
        "isByteArray" : false,
        "isBoolean" : false,
        "isDate" : false
      },
      "nameInCamelCase" : "Message",
      "iexclusiveMaximum" : false,
      "moreNonReadOnly" : false,
      "isPrimitiveType" : false,
      "isDateTime" : false,
      "isUuid" : false,
      "isDefault" : false,
      "isMapContainer" : false,
      "isListContainer" : true,
      "isMultipart" : false,
      "isResponseBinary" : false,
      "isResponseFile" : false,
      "isBinary" : false,
      "isFile" : false,
      "isEnum" : false,
      "isArrayModel" : false,
      "isAlias" : false,
      "isObject" : false,
      "hasInnerObject" : false,
      "isContainer" : true,
      "isNotContainer" : false,
      "isReadOnly" : false,
      "isCollectionFormatMulti" : false,
      "hasMore" : false,
      "isInteger" : false,
      "isNumber" : false,
      "hasHeaders" : false,
      "isString" : false,
      "isNumeric" : false,
      "isLong" : false,
      "isFloat" : false,
      "isDouble" : false,
      "isByteArray" : false,
      "isBoolean" : false,
      "isDate" : false
      } ]

To fix this, it would be needed to add the "importPath" or "filename" key to the nested Array/Object schema objects, but that fix would require a better knowledge of the Java code that I don't currently have.

Regardless this Pull Request helps the first level of type hinting so I reopened it.

@MalteEbner
Copy link

Would it be possible to already have a basic typehinting functionality for types like 'float', 'dict'... that just gives 'object' as a type hint instead of importing the more precise type?

@Dringho Dringho closed this Jan 26, 2021
@Dringho Dringho reopened this Jan 26, 2021
@Dringho
Copy link
Author

Dringho commented Jan 26, 2021

Would it be possible to already have a basic typehinting functionality for types like 'float', 'dict'... that just gives 'object' as a type hint instead of importing the more precise type?

I'm not sure I understand your idea. Why would you type hint something broader than the actual return? With this PR the basic typehints work as well as the complex ones (except for the nested complex types)

@MalteEbner
Copy link

Would it be possible to already have a basic typehinting functionality for types like 'float', 'dict'... that just gives 'object' as a type hint instead of importing the more precise type?

I'm not sure I understand your idea. Why would you type hint something broader than the actual return? With this PR the basic typehints work as well as the complex ones (except for the nested complex types)

If I understand it correctly we have 4 possibilities how many typehint functionality to have (listed in increasing value):

  1. None (like currently on master)
  2. Basic (like I proposed in my last comment)
  3. Basic + complex (like in your branch)
  4. Basic + complex + nested-complex (which would be best)

I assume that the branch is not merged because getting 4) to work is quite complex.
As I currently only have 1), I would prefer to have 2) or 3) merged, than trying to get 4) working.

@MalteEbner
Copy link

@Dringho Do you know why this PR is not approved? Does it need to be rebased on master or are there additional problems?

@Dringho
Copy link
Author

Dringho commented Feb 10, 2021

@MalteEbner I do not know why this Pull request has not been reviewed/approved. I'm not sure about the development cylce of this project, I'm just an outside contributor.
@HugoMario would you care to provide some feedback here?

@dangell7
Copy link

BUMP

@dangell7
Copy link

dangell7 commented Jan 13, 2022

Can someone rebase this? Who wants to make some $$?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants