Skip to content

Conversation

@JustKiddingCode
Copy link

@JustKiddingCode JustKiddingCode commented Jun 4, 2025

This pull request implements parsing docstrings via griffe and sets the relevant openapi descriptions to the corresponding docstring part.

The openapi has 3 description fields: the route itself, each parameter and the response.
With docstrings, you can only set one: The route itself. You can set the description for parameter and response using attributes in the router or annotations.
If I describe the function with all arguments, e.g. to build a documentation via mkdocs, the arguments that get autofilled by depends will appear in the openapi spec. If I want the description in the openapi spec and in the docstring there will be redundancy. Doing it only in the fastapi part, it will not be part of mkdocs (or similar).
As described by #3225, using docstrings is complicated, because the structure is not standardized. So I solve this using griffe, which can parse the most common formats (google, numpy, sphinx). Currently i set it to google format, but the plan is to make this configurable. However, if the description is set by fastapi attributes, this will take precedence.

@router.get(
    "/events/{eventId}",
    status_code=status.HTTP_200_OK,
    summary="Get event details",
)
@alog
async def get_event_details(
    auth: Annotated[Auth, Depends(authorize(AuthStrategy.HYBRID))],
    db: Annotated[AsyncSession, Depends(get_db)],
    event_id: Annotated[int | uuid.UUID, Path(alias="eventId")],
) -> AddEditGetEventResponse:
    """Retrieve details of a specific event either by its event ID, or via its UUID.

    args:
        auth: the user's authentification status
        db: the current database
        event_id: the ID or UUID of the event

    returns:
        the event details
    """
    return await _get_event_details(db, event_id, auth.user)

will be converted to:

image

Before i continue to work on this, i want to know if adding a dependency is ok. If so, i would allow to set the parse format of the docstring (griffe supports google, numpy and sphinx).

@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

📝 Docs preview for commit 0e5dca3 at: https://6c93d271.fastapitiangolo.pages.dev

@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

📝 Docs preview for commit 64f8856 at: https://3cd9cf8d.fastapitiangolo.pages.dev

@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

📝 Docs preview for commit 8683d33 at: https://c9dc173d.fastapitiangolo.pages.dev

@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

📝 Docs preview for commit 0226b66 at: https://99c0a14b.fastapitiangolo.pages.dev

@JustKiddingCode JustKiddingCode marked this pull request as ready for review June 4, 2025 14:33
@YuriiMotov

This comment was marked as outdated.

@JustKiddingCode

This comment was marked as outdated.

@YuriiMotov YuriiMotov changed the title Use griffe to parse docstrings and add descriptions ✨ Extract descriptions of parameters and responses from docstring Jun 4, 2025
@YuriiMotov YuriiMotov added the feature New feature or request label Jun 4, 2025
@HenriBlacksmith
Copy link

Just a personal opinion, I am not a maintainer but as a FastAPI user:

  • API documentation is already supported using OpenAPI, it does not make much sense to use mkdocs docs for API endpoints.
  • At most the new dependency must be optional, FastAPI is a micro framework and by default it should not pull tons of dependencies.
  • As for dependencies it looks this addition impacts the existing flow though I'll let more skilled reviewers checking that.

@JustKiddingCode
Copy link
Author

* API documentation is already supported using OpenAPI, it does not make much sense to use mkdocs docs for API endpoints.

Were using mkdocs with Modern MISP. Documenting only the API would leave out integral parts of the software, doing documentation mixed (openapi for api, rest mkdocs) would be error-prone. Also if I'm using api functions outside the api, how should i document them?

* At most the new dependency must be optional, FastAPI is a micro framework and by default it should not pull tons of dependencies.

I agree with the later part, however, making it optional would require much additional code and i doubt the benefit would outweigh it.

* As for dependencies it looks this addition impacts the existing flow though I'll let more skilled reviewers checking that.

Could you elaborate that? I don't understand it.

@YuriiMotov
Copy link
Member

@JustKiddingCode, let's first wait for Sebastián to look at this PR and validate the idea.
Then (in case of idea is accepted) we will need to add more tests and describe this feature in docs.

@github-actions github-actions bot added the conflicts Automatically generated when a PR has a merge conflict label Sep 29, 2025
@github-actions
Copy link
Contributor

This pull request has a merge conflict that needs to be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conflicts Automatically generated when a PR has a merge conflict feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants