[BUG][python-nextgen] Python generates incorrect parsing code for 2D arrays of objects

ianthetechie commented 1 year ago

Bug Report Checklist

[x] Have you provided a full/minimal spec to reproduce the issue?
[x] Have you validated the input using an OpenAPI validator (example)?
[ ] Have you tested with the latest master to confirm the issue still exists?
[x] Have you searched for related issues/PRs?
[x] What's the actual output vs expected output?
[ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Showing is probably easier than telling, so here's a commit in which I manually patched the invalid parsing code produced by the generator: https://github.com/stadiamaps/stadiamaps-api-py/commit/3d9a1050dbee8965fdf522ab23aac374222fbe9e#diff-bfcde497ac94587a9c9275048d15ca3a1a4aec8dc206e56b1d0133c113dab4aeR113.

The Python(-nextgen) API generator produces incorrect parsing code for 2D array of objects. The generated code attempts to invoke the from_dict method on a list which fails for obvious reasons. It should instead produce code something along the lines of my modified code.

openapi-generator version

v6.6.0 (homebrew CLI on macOS)

OpenAPI declaration file content or url

Stadia Maps OpenAPI spec: https://api.stadiamaps.com/openapi.yaml.

Generation Details

Generated using the following CLI invocation. I am using openapi-generator v6.6.0 installed via homebrew on macOS Ventura.

openapi-generator generate -i https://api.stadiamaps.com/openapi.yaml -g python-nextgen --strict-spec=true -o stadiamaps-python-api -p disallowAdditionalPropertiesIfNotPresent=false -p packageName=stadiamaps -p packageUrl=https://docs.stadiamaps.com/ -p packageVersion=0.5.0

Steps to reproduce

Run the CLI invocation above.
Try to use the code. I have included a trivial example below that doesn't require any API keys; just the generated code.

import stadiamaps

obj = {
  "units": "kilometers",
  "sources": [
    [
      {
        "lon": -73.990508,
        "lat": 40.744014
      }
    ]
  ],
  "targets": [
    [
      {
        "lon": -73.990508,
        "lat": 40.744014
      },
      {
        "lon": -73.979713,
        "lat": 40.739735
      },
      {
        "lon": -73.985015,
        "lat": 40.752522
      },
      {
        "lon": -73.983704,
        "lat": 40.750117
      },
      {
        "lon": -73.993519,
        "lat": 40.750552
      }
    ]
  ],
  "sources_to_targets": [
    [
      {
        "distance": 0,
        "time": 0,
        "to_index": 0,
        "from_index": 0
      },
      {
        "distance": 1.126,
        "time": 829,
        "to_index": 1,
        "from_index": 0
      },
      {
        "distance": 1.327,
        "time": 969,
        "to_index": 2,
        "from_index": 0
      },
      {
        "distance": 1.134,
        "time": 840,
        "to_index": 3,
        "from_index": 0
      },
      {
        "distance": 1.355,
        "time": 986,
        "to_index": 4,
        "from_index": 0
      }
    ]
  ]
}

stadiamaps.MatrixResponse.from_dict(obj)  # Throws an exception

Related issues/PRs

None that I'm aware of

Suggest a fix

I am not very much aware of the internals of the OpenAPI Generator codebase, but it seems that this is a Python-specific issue. Both TypeScript and Kotlin generated clients work fine. I assume there is some sort of recursive generation that happens, and it apparently views List[Coordinate] as a distinct type, but this is not correct in Python, as it is just a plain old list under the hood. It should instead create (IMO) further nested list comprehensions of the sort I have hand-coded in the commit linked above.

wing328 commented 1 year ago

thanks for reporting the issue. I think it's a bug

let me know if you can contribute a PR or sponsor a fix. Thank you.

ianthetechie commented 1 year ago

I would be happy to contribute a PR if you can point me in the right direction (file[s] where this code gets generated should be good enough for me to take it from there)

wing328 commented 1 year ago

https://github.com/OpenAPITools/openapi-generator/pull/15844 is a good reference.

PM me via Slack if you need help.

Thanks for offering help to fix this issue.

wing328 commented 1 year ago

@ianthetechie @japrescott can you please give it a try with the latest master? SNAPSHOT version can be found in project's README.

If I remember correctly, we did recently merge a fix for that.

(python-nextgen was renamed to python in the latest master)

OpenAPITools / openapi-generator