Skip to content

docs: Add docstrings to all model classes #261

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 114 additions & 1 deletion folksonomy/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,37 @@


class User(BaseModel):
"""
User model for authentication and authorization.

Represents the currently authenticated user in the API.
Used for permission checking and tracking tag ownership.

Attributes:
user_id: The identifier of the user, or None for unauthenticated requests
Comment on lines +19 to +20
Copy link
Member

Choose a reason for hiding this comment

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

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.

"""
user_id: Optional[str]


class ProductTag(BaseModel):
"""
Core model representing a product property/tag in the folksonomy system.

Contains a key-value pair associated with a product, along with metadata about
the tag's ownership, versioning, and editing history.

Used in tag creation, update, retrieval, and deletion operations.

Attributes:
product: The barcode of the product (digits only)
Copy link
Member

Choose a reason for hiding this comment

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

I would remove the (digits only) because we don't enforce it, and it may change in the future…

Suggested change
product: The barcode of the product (digits only)
product: The barcode of the product

k: The property key (must match permitted character pattern)
v: The property value
owner: The user_id of the owner ("" for public tags)
version: Integer version number (starts at 1)
editor: The user_id of the last editor
last_edit: Timestamp of the last edit
comment: Optional comment about the tag
"""
product: str
k: str
v: str
Expand Down Expand Up @@ -57,18 +84,104 @@ def version_check(cls, version):


class ProductStats(BaseModel):
"""
Statistics model for a product's tags/properties.

Used by the /products/stats endpoint to provide a summary of tagging activity
for each product.

Attributes:
product: The barcode of the product
keys: Number of distinct keys/properties for this product
editors: Number of distinct users who edited the product's tags
last_edit: Timestamp of the most recent edit for any tag on this product
"""
product: str
keys: int
editors: int
last_edit: datetime


class ProductList(BaseModel):
"""
Simplified product tag model used for product listings.

Used by the /products endpoint to provide a list of products that have
specific tags/properties.

Attributes:
product: The barcode of the product
k: The property key
v: The property value
"""
product: str
k: str
v: str

class KeyStats(BaseModel):
"""
Statistics model for property keys.

Used by the /keys endpoint to provide information about how frequently
different keys are used across products in the folksonomy system.

Attributes:
k: The property key
count: Number of products using this key
values: Number of distinct values associated with this key
"""
k: str
count: int
values: int
values: int


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
Comment on lines +138 to +187
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

Copy link
Member

Choose a reason for hiding this comment

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

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…)