Skip to content

feat: Add parameter description to properties #345

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

Closed
wants to merge 1 commit into from

Conversation

alexifm
Copy link
Contributor

@alexifm alexifm commented Mar 10, 2021

This is draft to propose adding the parameter description from the OpenAPI schema to the properties based on the comment here: #224 (comment). This proves useful if it's coupled with a mkdocs template using mkdocstrings so that the parameter descriptions in the OpenAPI spec can be included in the docs.

Still requires tests and formatting/cleanup but feedback on if there is interest in this and it's taking the right approach would be appreciated.

This PR could be expanded to include the template and method to handle mkdocs.

@dbanty
Copy link
Collaborator

dbanty commented Mar 13, 2021

Awesome, thanks! So the idea is store all the various description values we get for different properties/operations and stick those in docstrings in the generated code right? I'm on board! Probably better to take this one step at a time since there will be a great many places to stick docstrings. Some questions we'll need to answer:

  1. Should we generate the same string to put on sync and async functions?
  2. Should we put a description at the module level for operations too?
  3. Do we document responses as well?
  4. What about generated models / enums? And how best to document their properties (fairly certain mkdocs has a convention for that)?

I will say that I prefer the Google-style docstrings, so we should use those. Might be easiest to just start with descriptions and params for endpoints, then include models in a follow-up PR? Up to you though if you want to just go for it.

@alexifm
Copy link
Contributor Author

alexifm commented May 3, 2021

Hey @dbanty sorry for the slow going on this.

So the idea is store all the various description values we get for different properties/operations and stick those in docstrings in the generated code right?

Yup. The way I have done this in the fork I have right now is simple: I implemented the code in this PR you're seeing and then added a mkdocs stage in the builder as well as using a custom template.

Probably better to take this one step at a time since there will be a great many places to stick docstrings.

Right, my thinking was this PR could be simple in that it doesn't change much other than ensure the description is in place for when docstrings are desired.

Here is how we did the docstrings on the sync function. This template:
https://github.com/triaxtec/openapi-python-client/blob/890411a79772592b369ab18695e92fe92b402eb1/openapi_python_client/templates/endpoint_module.py.jinja#L114

was updated to this:

    """
    {{ endpoint.description }} 
    
    Args:
    {% for parameter in endpoint.query_parameters %}
        {{ parameter.python_name }}: {{ parameter.description }}
    {% endfor %}

    Returns:
        Either a valid response or an unauthorized, not found or other error.
    """

@dbanty
Copy link
Collaborator

dbanty commented Oct 17, 2021

I think #505 is attempting the same general idea as this, so closing in favor of that one.

@dbanty dbanty closed this Oct 17, 2021
@alexifm alexifm deleted the feat/description branch December 17, 2021 15:47
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.

2 participants