Warn if widgets are used as class variables. #130
Conversation
WalkthroughThe Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (3)
- magicclass/init.py (1 hunks)
- magicclass/_gui/_base.py (2 hunks)
- tests/test_construction.py (2 hunks)
Files skipped from review due to trivial changes (2)
- magicclass/init.py
- tests/test_construction.py
Additional comments: 2
magicclass/_gui/_base.py (2)
83-83: The import of
FieldGrouphas been added correctly.722-723: The check for a string "separator" and its conversion to a
Separatorwidget is correct.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- rst/main/images/fig_1-5.png
Files selected for processing (2)
- magicclass/_gui/_base.py (5 hunks)
- rst/main/quick_start.rst (2 hunks)
Additional comments: 4
magicclass/_gui/_base.py (2)
83-83: The addition of the
FieldGroupimport aligns with the PR objective and the AI-generated summary.83-83: According to the AI-generated summary, the
fieldandvfieldmethods have been removed. Ensure that this change does not affect any other parts of the codebase that may rely on these methods.rst/main/quick_start.rst (2)
93-94: The updated note correctly reflects the warning system implemented for using widgets as class variables. It directs users to the appropriate documentation for using
fieldfunctions.264-269: The removal of the "Make Document of Your Widgets Automatically" section is not visible in the provided hunks, but based on the AI-generated summary, it appears to have been removed to align with the PR objectives.
| elif isinstance(attr, Widget) and not isinstance(attr, FieldGroup): | ||
| warnings.warn( | ||
| f"Widget {name!r} is given as a class attribute. This is not " | ||
| "recommended, as it is shared between all the instances. " | ||
| "Please use a field or vfield instead.", | ||
| UserWarning, | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
The warning message in _convert_an_attribute_into_widget now advises against using widgets as class variables, which aligns with the PR objective. However, the previous suggestion to include an example in the warning message has not been implemented. Consider adding an example to the warning message for clarity.
- "Please use a field or vfield instead.",
+ "Please use a field or vfield instead. For example, instead of 'my_widget = Widget()', use 'my_widget = field(Widget())'.",Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| elif isinstance(attr, Widget) and not isinstance(attr, FieldGroup): | |
| warnings.warn( | |
| f"Widget {name!r} is given as a class attribute. This is not " | |
| "recommended, as it is shared between all the instances. " | |
| "Please use a field or vfield instead.", | |
| UserWarning, | |
| stacklevel=2, | |
| ) | |
| elif isinstance(attr, Widget) and not isinstance(attr, FieldGroup): | |
| warnings.warn( | |
| f"Widget {name!r} is given as a class attribute. This is not " | |
| "recommended, as it is shared between all the instances. " | |
| "Please use a field or vfield instead. For example, instead of 'my_widget = Widget()', use 'my_widget = field(Widget())'.", | |
| UserWarning, | |
| stacklevel=2, | |
| ) |
The __init_subclass__ method in BaseGui is empty and does not seem to serve any purpose. Consider removing it if it's not intended to override any behavior.
- def __init_subclass__(cls, **kwargs):
- passCommittable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| elif isinstance(attr, Widget) and not isinstance(attr, FieldGroup): | |
| warnings.warn( | |
| f"Widget {name!r} is given as a class attribute. This is not " | |
| "recommended, as it is shared between all the instances. " | |
| "Please use a field or vfield instead.", | |
| UserWarning, | |
| stacklevel=2, | |
| ) |
In the __init_subclass__ method of MagicTemplate, ensure that super().__init_subclass__(**kwargs) is called to maintain proper inheritance behavior.
def __init_subclass__(cls, **kwargs):
+ super().__init_subclass__(**kwargs)
check_override(cls)Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| elif isinstance(attr, Widget) and not isinstance(attr, FieldGroup): | |
| warnings.warn( | |
| f"Widget {name!r} is given as a class attribute. This is not " | |
| "recommended, as it is shared between all the instances. " | |
| "Please use a field or vfield instead.", | |
| UserWarning, | |
| stacklevel=2, | |
| ) | |
| elif isinstance(attr, Widget) and not isinstance(attr, FieldGroup): | |
| super().__init_subclass__(**kwargs) | |
| warnings.warn( | |
| f"Widget {name!r} is given as a class attribute. This is not " | |
| "recommended, as it is shared between all the instances. " | |
| "Please use a field or vfield instead.", | |
| UserWarning, | |
| stacklevel=2, | |
| ) |
Consider adding an __init_subclass__ method to ContainerLikeGui for consistency and to ensure proper inheritance behavior, especially if check_override or other subclass initialization logic is required.
Consider adding an __init_subclass__ method to ContainerLikeGui for consistency and to ensure proper inheritance behavior, especially if check_override or other subclass initialization logic is required.
Consider adding an __init_subclass__ method to ContainerLikeGui for consistency and to ensure proper inheritance behavior, especially if check_override or other subclass initialization logic is required.
Related to #110.
Widgets should always be used inside fields.
Summary by CodeRabbit
fieldfunction to create widgets in magic classes.Menuclass for improved functionality.