-
Notifications
You must be signed in to change notification settings - Fork 0
docs: Document template lifecycle and refine #10
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
base: satisfy-linters-and-type-checkers
Are you sure you want to change the base?
Conversation
tagline - Expand README with definition, proxy vs non-proxy lifecycle, examples, and guidance - Update project description to "Type-safe templating for python"
Summary of ChangesHello @srnnkls, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily enhances the documentation for the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request significantly improves the documentation by adding a detailed explanation of the template lifecycle, which is a great addition for users of the library. The new content is well-structured and clarifies the differences between using and not using SerializationProxy. I've provided a couple of suggestions to make the code examples in the new documentation self-contained and runnable, which should improve the user experience.
| from pydantic import PlainSerializer | ||
| from typing import Annotated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code example is missing some necessary imports (dataclass and template). To make it easier for users to copy and run the code, it's best to include all required imports.
| from pydantic import PlainSerializer | |
| from typing import Annotated | |
| from dataclasses import dataclass | |
| from deigma import template | |
| from pydantic import PlainSerializer | |
| from typing import Annotated |
| @template("{{ user }}", use_proxy=False) | ||
| class UserTemplate: | ||
| user: User | ||
|
|
||
| t = UserTemplate(user=User(first_name="Li", last_name="Si")) | ||
| # At this point, nothing is serialized yet | ||
|
|
||
| str(t) # Serialization happens here | ||
| # If you call str(t) again, serialization runs again | ||
|
|
||
| # Note: In templates with loops/conditionals, you lose type info: | ||
| @template( | ||
| """ | ||
| {% for keyword in keywords %} | ||
| - {{ keyword.name }} {# keyword is now a plain dict, not SQLKeyword #} | ||
| {% endfor %} | ||
| """, | ||
| use_proxy=False | ||
| ) | ||
| class ListTemplate: | ||
| keywords: list[SQLKeyword] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example uses User and SQLKeyword types without defining them, and is also missing the import for @template. This makes the example incomplete and could be confusing for readers. I suggest adding minimal definitions for these types and the necessary import to make the snippet self-contained and runnable.
from dataclasses import dataclass
from deigma import template
# Assuming User and SQLKeyword are defined as dataclasses for this example
@dataclass
class User:
first_name: str
last_name: str
@dataclass
class SQLKeyword:
name: str
description: str
@template("{{ user }}", use_proxy=False)
class UserTemplate:
user: User
t = UserTemplate(user=User(first_name="Li", last_name="Si"))
# At this point, nothing is serialized yet
str(t) # Serialization happens here
# If you call str(t) again, serialization runs again
# Note: In templates with loops/conditionals, you lose type info:
@template(
"""
{% for keyword in keywords %}
- {{ keyword.name }} {# keyword is now a plain dict, not SQLKeyword #}
{% endfor %}
""",
use_proxy=False
)
class ListTemplate:
keywords: list[SQLKeyword]
tagline