-
Notifications
You must be signed in to change notification settings - Fork 0
chore: bump version to 0.5.5 and refactor modal and renderer components to remove Pydantic dependency #58
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
Conversation
WalkthroughThe changes involve updating the version number from "0.5.4" to "0.5.5" in both the JavaScript ( ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
py/gooey_gui/core/renderer.py (2)
29-38
: LGTM: Solid refactoring from Pydantic to plain Python class.The conversion maintains the same interface while removing the Pydantic dependency. The
__init__
method correctly handles default values forprops
andchildren
parameters.Consider adding a class docstring to address the static analysis hint:
class RenderTreeNode: + """Represents a node in the render tree with name, props, and children.""" def __init__(
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 29-29: Missing class docstring
(C0115)
44-49
: LGTM: Efficient recursive serialization method.The
to_dict()
method correctly handles recursive conversion of the render tree structure.Consider using a dictionary literal instead of
dict()
call as suggested by static analysis:- def to_dict(self) -> dict: - return dict( - name=self.name, - props=self.props, - children=[child.to_dict() for child in self.children], - ) + def to_dict(self) -> dict: + return { + "name": self.name, + "props": self.props, + "children": [child.to_dict() for child in self.children], + }🧰 Tools
🪛 Pylint (3.3.7)
[convention] 44-44: Missing function or method docstring
(C0116)
[refactor] 45-49: Consider using '{"name": self.name, "props": self.props, "children": [child.to_dict() for child in self.children], ... }' instead of a call to 'dict'.
(R1735)
py/gooey_gui/components/modal.py (2)
5-8
: LGTM: Clean conversion from Pydantic to plain Python class.The
AlertDialogRef
class maintains the same interface while removing Pydantic dependency. The__init__
method correctly initializes thekey
andis_open
attributes with appropriate defaults.Consider adding a class docstring to improve documentation:
class AlertDialogRef: + """Reference to an alert dialog with state management capabilities.""" def __init__(self, key: str, is_open: bool = False):
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 5-5: Missing class docstring
(C0115)
25-27
: LGTM: Elegant solution for maintaining compatibility.The
from_alert_dialog
class method provides a clean way to create aConfirmDialogRef
from anAlertDialogRef
while preserving the existing interface.Consider adding a docstring to document the conversion method:
@classmethod def from_alert_dialog(cls, ref: AlertDialogRef) -> "ConfirmDialogRef": + """Create a ConfirmDialogRef from an existing AlertDialogRef.""" return cls(key=ref.key, is_open=ref.is_open)
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 26-26: Missing function or method docstring
(C0116)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
py/poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
package.json
(1 hunks)py/gooey_gui/components/modal.py
(3 hunks)py/gooey_gui/core/renderer.py
(2 hunks)py/pyproject.toml
(1 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
py/gooey_gui/components/modal.py
[convention] 5-5: Missing class docstring
(C0115)
[convention] 26-26: Missing function or method docstring
(C0116)
py/gooey_gui/core/renderer.py
[convention] 29-29: Missing class docstring
(C0115)
[convention] 40-40: Missing function or method docstring
(C0116)
[convention] 44-44: Missing function or method docstring
(C0116)
[refactor] 45-49: Consider using '{"name": self.name, "props": self.props, "children": [child.to_dict() for child in self.children], ... }' instead of a call to 'dict'.
(R1735)
[convention] 52-52: Missing class docstring
(C0115)
🔇 Additional comments (6)
py/pyproject.toml (2)
3-3
: LGTM: Version bump is consistent with PR objectives.The version increment to 0.5.5 aligns with the PR objectives and matches the corresponding change in package.json.
8-16
: LGTM: Pydantic dependency correctly removed.The removal of the
pydantic
dependency is consistent with the refactoring of Pydantic models to plain Python classes in the renderer and modal components. This simplifies the project dependencies.package.json (1)
3-3
: LGTM: Version bump maintains consistency across projects.The version increment to 0.5.5 correctly maintains version synchronization between the JavaScript and Python projects.
py/gooey_gui/core/renderer.py (2)
53-53
: LGTM: Appropriate type annotation update.The explicit
None
type annotation correctly reflects the optional parameter behavior.
152-152
: LGTM: Correct usage of new serialization method.The update to use
root.to_dict()["children"]
properly integrates with the refactoredRenderTreeNode
class while maintaining the same JSON response structure.py/gooey_gui/components/modal.py (1)
38-38
: LGTM: Correct usage of new class method.The update properly uses the new
from_alert_dialog
class method instead of Pydantic'sparse_obj
, maintaining the same functionality while removing the Pydantic dependency.
…ts to remove Pydantic dependency
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
py/gooey_gui/core/renderer.py (1)
44-49
: Consider using dict literal syntax and adding documentation.The
to_dict()
method correctly implements recursive serialization, but could benefit from minor improvements:Apply this diff to use dict literal syntax as suggested by static analysis:
- def to_dict(self) -> dict: - return dict( - name=self.name, - props=self.props, - children=[child.to_dict() for child in self.children], - ) + def to_dict(self) -> dict: + """Convert the node and its children to a dictionary representation.""" + return { + "name": self.name, + "props": self.props, + "children": [child.to_dict() for child in self.children], + }🧰 Tools
🪛 Pylint (3.3.7)
[convention] 44-44: Missing function or method docstring
(C0116)
[refactor] 45-49: Consider using '{"name": self.name, "props": self.props, "children": [child.to_dict() for child in self.children], ... }' instead of a call to 'dict'.
(R1735)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.json
is excluded by!**/package-lock.json
py/poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
package.json
(1 hunks)py/gooey_gui/components/modal.py
(3 hunks)py/gooey_gui/core/renderer.py
(2 hunks)py/pyproject.toml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- py/pyproject.toml
🧰 Additional context used
🪛 Pylint (3.3.7)
py/gooey_gui/components/modal.py
[convention] 5-5: Missing class docstring
(C0115)
[convention] 26-26: Missing function or method docstring
(C0116)
py/gooey_gui/core/renderer.py
[convention] 29-29: Missing class docstring
(C0115)
[convention] 40-40: Missing function or method docstring
(C0116)
[convention] 44-44: Missing function or method docstring
(C0116)
[refactor] 45-49: Consider using '{"name": self.name, "props": self.props, "children": [child.to_dict() for child in self.children], ... }' instead of a call to 'dict'.
(R1735)
[convention] 52-52: Missing class docstring
(C0115)
🔇 Additional comments (6)
py/gooey_gui/core/renderer.py (3)
29-38
: LGTM! Well-implemented Pydantic replacement.The refactor from Pydantic BaseModel to plain Python class is correctly implemented. The constructor properly handles optional parameters with sensible defaults and attribute initialization is clean.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 29-29: Missing class docstring
(C0115)
53-53
: LGTM! Proper type annotation update.The type annotation correctly allows
None
values, which aligns with the default parameter handling in the constructor.
152-152
: LGTM! Correct serialization usage.The change to use
root.to_dict()["children"]
ensures proper serialization of the render tree nodes instead of directly accessing the children list. This maintains consistency with the new plain Python class approach.py/gooey_gui/components/modal.py (3)
5-8
: LGTM! Clean Pydantic replacement.The refactor from Pydantic model to plain Python class is well-implemented. The constructor correctly initializes the required attributes with appropriate defaults.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 5-5: Missing class docstring
(C0115)
25-27
: LGTM! Well-designed factory method.The
from_alert_dialog
class method provides a clean way to create aConfirmDialogRef
instance from an existingAlertDialogRef
, properly copying the relevant attributes. This replaces the previous Pydanticparse_obj
approach effectively.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 26-26: Missing function or method docstring
(C0116)
38-38
: LGTM! Correct usage of the new factory method.The function correctly uses the new
from_alert_dialog
class method instead of the previous Pydantic parsing approach. This maintains the same functionality while removing the Pydantic dependency.
No description provided.