-
Notifications
You must be signed in to change notification settings - Fork 672
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
Add solution #679
base: master
Are you sure you want to change the base?
Add solution #679
Conversation
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.
Your code meets the task requirements and constraints effectively, and the core functionality is intact. While there are some minor type annotation issues, they do not significantly impact the functionality. Great job! 🎉 Keep up the good work and consider refining those annotations for clarity and precision in future projects.
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
self.min_amount = min_amount | ||
self.max_amount = max_amount | ||
|
||
def __set_name__(self, owner: Visitor, name: str) -> None: |
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.
The __set_name__
method's owner
parameter is annotated with Visitor
, but it should be type
since owner
is the class where the descriptor is used. Consider changing the annotation to type
.
self.public_name = name | ||
self.protected_name = "_" + name | ||
|
||
def __get__(self, instance: Visitor, owner: Visitor) -> int: |
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.
The __get__
method's parameters instance
and owner
are annotated with Visitor
, but owner
should be type
. Consider updating the annotation for owner
to type
.
def __get__(self, instance: Visitor, owner: Visitor) -> int: | ||
return getattr(instance, self.protected_name) | ||
|
||
def __set__(self, instance: Visitor, value: Visitor) -> None: |
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.
The __set__
method's parameters instance
and value
are annotated with Visitor
, but value
should be int
as it represents the value being set. Consider updating the annotation for value
to int
.
|
||
|
||
class Slide: | ||
pass | ||
def __init__( | ||
self, name: str, limitation_class: SlideLimitationValidator |
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.
The limitation_class
parameter in the Slide
class constructor is annotated with SlideLimitationValidator
, but it should be type[SlideLimitationValidator]
to indicate that it expects a class, not an instance.
Add solution