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.

Sign up for GitHub

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

Jump to bottom

[Python] Fixed docstrings in api.mustache #6391

Merged
merged 7 commits into from
Jun 4, 2020
Merged

[Python] Fixed docstrings in api.mustache #6391

merged 7 commits into from
Jun 4, 2020

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented May 22, 2020 •
edited
Loading

Fixes swagger-api/swagger-codegen#9630

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.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@wing328
Copy link
Member

wing328 commented May 22, 2020

@Ark-kun thanks for the PR.

We also have a new python-experimental client generator, which has better support for new features in OAS v3 such as oneOf, allOf, etc

You may want to give it a try and file a similar fix to the python-experimental client generator.

@Ark-kun
Copy link
Contributor Author

Ark-kun commented May 22, 2020 •
edited
Loading

We also have a new python-experimental client generator, which has better support for new features in OAS v3 such as oneOf, allOf, etc

Sounds great.

You may want to give it a try and file a similar fix to the python-experimental client generator.

Done. It's probably OK to do that in one PR.

@wing328
Copy link
Member

wing328 commented May 22, 2020

Can you please also run the following to update the OAS v3 samples?

./bin/openapi3/python-petstore.sh
./bin/openapi3/python-experimental-petstore.sh

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

@wing328 wing328 added this to the 5.0.0 milestone May 22, 2020
@ghost
Copy link

ghost commented May 22, 2020

@Ark-kun

I have checked the documentation for sphinx (https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html)

:param [ParamName]: [ParamDescription], defaults to [DefaultParamVal]
:type [ParamName]: [ParamType](, optional)

I did not find the format that is used in this PR in the docs, please do let me know if this format is mentioned somewhere. Can we use the format that is mentioned in the doc instead ?

@spacether
Copy link
Contributor

spacether commented May 22, 2020 •
edited
Loading

@Ark-kun

I have checked the documentation for sphinx (https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html)

:param [ParamName]: [ParamDescription], defaults to [DefaultParamVal]
:type [ParamName]: [ParamType](, optional)

I did not find the format that is used in this PR in the docs, please do let me know if this format is mentioned somewhere. Can we use the format that is mentioned in the doc instead ?

We do not have a standardized docstring format yet but sphinx is fine.

@spacether
Copy link
Contributor

spacether commented May 22, 2020 •
edited
Loading

We also have a new python-experimental client generator, which has better support for new features in OAS v3 such as oneOf, allOf, etc

Sounds great.

You may want to give it a try and file a similar fix to the python-experimental client generator.

Done. It's probably OK to do that in one PR.

@Ark-kun there are many many docstrings in python-experimental including:

  • docstrings in model_utils
  • docstrings in api.pi
  • docstrings in every model
  • many others

Also in python-experimental we use our data type to define types that python accepts
So if a model has a property that accepts list of list of str that data type in python-experimental will be [[str]]. Fixing this in python-experimental will likely require adding a new property that is stored alongside dataType, maybe documentationDataType in the java layer.

How about addressing those larger changes in python-experimental changes as a separate PR?
Fixing only api.mustache for python-experimental is fine here.

@spacether spacether changed the title [Python] Fixed docstrings [Python] Fixed docstrings in api.mustache May 22, 2020
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Jun 3, 2020

I did not find the format that is used in this PR in the docs, please do let me know if this format is mentioned somewhere. Can we use the format that is mentioned in the doc instead ?

I've amended my changes to comply with the doc you've linked.

Can you please take another look?

Thanks.

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Jun 3, 2020

Can you please also run the following to update the OAS v3 samples?

./bin/openapi3/python-petstore.sh
./bin/openapi3/python-experimental-petstore.sh

Done.

Some files there might have been stale, so they are now modified even though they are unrelated to this PR.

spacether
spacether approved these changes Jun 4, 2020
View reviewed changes
Copy link
Contributor

@spacether spacether left a comment

Choose a reason for hiding this comment

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

This looks good. Thank you for this PR!

@spacether spacether merged commit 6f6c8ed into OpenAPITools:master Jun 4, 2020
jimschubert added a commit that referenced this pull request Jun 5, 2020
@jimschubert
Merge branch 'master' into bugfix/4999
92b403d 
* master: (345 commits)
  [kotlin][spring] Fix ApiUtil compilation (#6084)
  update python samples
  [Python] Fixed docstrings in api.mustache (#6391)
  [BUG][python] Support named arrays (#6493)
  [Go] whitelist AdditionalProperties in the field name (#6543)
  [kotlin][client] remove tabs usage (#6526)
  [PS] automatically derive discriminator mapping for oneOf/anyOf  (#6542)
  [C++][Ue4] various bus fixes (#6539)
  Fix incorrect npx command (#6537)
  update pester to 5.x (#6536)
  comment out openapi3 java jersey2-java8 tests
  add additional properties support to powershell client generator (#6528)
  [Go][Experimental] Support additionalProperties (#6525)
  #5476 [kotlin] [spring] fix swagger and spring annotation for defaultValue (#6101)
  [samples] regenerate (#6533)
  [python] Fix date-time parsing (#6458)
  Register OAuth2ClientContext as bean (#6172)
  [Go][Experimental] Fix discriminator lookup (#6521)
  Typescript-rxjs: print param name (#6368)
  add oneof discrimistrator lookup to go experimental (#6517)
  ...
jimschubert added a commit that referenced this pull request Jun 6, 2020
@jimschubert
Merge branch 'master' into ensures
9ddc45c 
* master:
  Fix typescript generator for parameter collectionFormat for pipes ssv (#6553)
  [C++][Pistache] Catch HttpError from user-provided handler (#6520)
  remove scala related profile from the pom (#6554)
  move ruby tests to travis (#6555)
  [Java][jersey2] fix cast error for default value in DateTimeOffset object (#6547)
  [Swift] fix GET request with array parameter (#6549)
  [kotlin][spring] Fix ApiUtil compilation (#6084)
  update python samples
  [Python] Fixed docstrings in api.mustache (#6391)
  [BUG][python] Support named arrays (#6493)
  [Go] whitelist AdditionalProperties in the field name (#6543)
  [kotlin][client] remove tabs usage (#6526)
  [PS] automatically derive discriminator mapping for oneOf/anyOf  (#6542)
  [C++][Ue4] various bus fixes (#6539)
  Fix incorrect npx command (#6537)
  update pester to 5.x (#6536)
  comment out openapi3 java jersey2-java8 tests
  add additional properties support to powershell client generator (#6528)
  [Go][Experimental] Support additionalProperties (#6525)
  #5476 [kotlin] [spring] fix swagger and spring annotation for defaultValue (#6101)
@Bobgy Bobgy mentioned this pull request Jun 19, 2020
Closed
Bobgy added a commit to Bobgy/pipelines that referenced this pull request Jun 22, 2020
@Bobgy
update templates according to OpenAPITools/openapi-generator/pull/6391
c86c59a
@Bobgy Bobgy mentioned this pull request Jun 22, 2020
Merged
1 task
k8s-ci-robot pushed a commit to kubeflow/pipelines that referenced this pull request Jun 23, 2020
@Bobgy
doc(sdk/client): fix kfp-server-api py client's docstring format (#4047)
f456ee9 
* Pull templates from upstream 4.3.1

* update templates according to OpenAPITools/openapi-generator/pull/6391

* regenerate python client
Bobgy added a commit to kubeflow/pipelines that referenced this pull request Jul 2, 2020
@Bobgy
doc(sdk/client): fix kfp-server-api py client's docstring format (#4047)
91d4e50 
* Pull templates from upstream 4.3.1

* update templates according to OpenAPITools/openapi-generator/pull/6391

* regenerate python client
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this pull request Dec 9, 2020
@Bobgy @Jeffwan
doc(sdk/client): fix kfp-server-api py client's docstring format (kub…
0684fdb 
…eflow#4047)

* Pull templates from upstream 4.3.1

* update templates according to OpenAPITools/openapi-generator/pull/6391

* regenerate python client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spacether spacether spacether approved these changes

Assignees
No one assigned
Labels
Client: Python Enhancement: Feature Feature: Documentation
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

Docs strings for python aren't parsed properly by sphinx
3 participants
@Ark-kun @wing328 @spacether