docs: Add docstrings to all model classes#261
Conversation
|
I need some clarification with the usage of "tags" and "property" within the ProductTag model since the usage of these terms isn't consistent. I would assume a product tag would just be a list of tags, but the /product/{product} endpoint, which uses the ProductTag model, returns properties. This is confusing not because of what it's returning but because of the terminology. I am not sure if this is something that can or should be fixed, but it has the potential to confuse new contributors. For this PR, I assumed that the two terms are interchangeable. Appreciate any feedback/comments and edit suggestions! |
| Attributes: | ||
| user_id: The identifier of the user, or None for unauthenticated requests |
There was a problem hiding this comment.
It's not the way to document in pydantic.
You should use https://docs.pydantic.dev/2.9/concepts/fields/#customizing-json-schema
with annotated (https://docs.pydantic.dev/2.9/concepts/fields/#using-annotated)
This way we will have it in the OpenAPI documentation.
Please change it in all the models below.
| Used in tag creation, update, retrieval, and deletion operations. | ||
|
|
||
| Attributes: | ||
| product: The barcode of the product (digits only) |
There was a problem hiding this comment.
I would remove the (digits only) because we don't enforce it, and it may change in the future…
| product: The barcode of the product (digits only) | |
| product: The barcode of the product |
| class HelloResponse(BaseModel): | ||
| """ | ||
| Response model for the root endpoint (/). | ||
|
|
||
| Contains a welcome message to introduce users to the Folksonomy API. | ||
|
|
||
| Attributes: | ||
| message: A string containing the welcome message | ||
| """ | ||
| message: str | ||
|
|
||
|
|
||
| class TokenResponse(BaseModel): | ||
| """ | ||
| Response model for authentication endpoints (/auth and /auth_by_cookie). | ||
|
|
||
| Represents the OAuth2 token response returned after successful authentication. | ||
|
|
||
| Attributes: | ||
| access_token: The bearer token to be used for authenticated requests | ||
| token_type: The type of token | ||
| """ | ||
| access_token: str | ||
| token_type: str | ||
|
|
||
|
|
||
| class PingResponse(BaseModel): | ||
| """ | ||
| Response model for the health check endpoint (/ping). | ||
|
|
||
| Used to verify that the API server is running and can connect to the database. | ||
|
|
||
| Attributes: | ||
| ping: A string containing "pong" followed by the current timestamp | ||
| """ | ||
| ping: str | ||
|
|
||
|
|
||
| class ValueCount(BaseModel): | ||
| """ | ||
| Response model for the unique values endpoint (/values/{k}). | ||
|
|
||
| Represents a unique value for a given property and the count of products using it. | ||
|
|
||
| Attributes: | ||
| v: The property value | ||
| product_count: Number of products that use this value for the specified property | ||
| """ | ||
| v: str | ||
| product_count: int No newline at end of file |
There was a problem hiding this comment.
It's a bit strange to introduce those classes if we don't use them yet.
You might introduce them in api, but using alternatives to avoid changing the API (the response might be of type: str | ValueCount for example.
There was a problem hiding this comment.
Hi, I wanted to clarify your feedback regarding introducing the new models (e.g., HelloResponse, TokenResponse, PingResponse, ValueCount). When you mentioned using alternatives like str | ValueCount, do you mean that the API should support both the old response format (e.g. plain strings) and the new model based format for backward compatibility?
For example, should the /values/{k} endpoint return either a string or a ValueCount object depending on the context? Or are you suggesting a different approach? I want to make sure I fully understand your expectations before proceeding.
Thank you!
There was a problem hiding this comment.
For example, should the /values/{k} endpoint return either a string or a ValueCount object depending on the context?
Yes
(sorry @suchithh I was AFK for more than a week…)
4 participants
What
This PR adds docstrings to all model classes (including Product models). Follow-up to comment left by @alexgarel (#238 (review)) to PR #238
The docstrings now provide:
The following recently added models now have comprehensive docstrings:
Along with previously existing models: