-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
'My_Solution' #1510
base: master
Are you sure you want to change the base?
'My_Solution' #1510
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.
Thank you for your hard work on implementing the caching decorator! 😊 However, there are a couple of critical issues that need to be addressed before we can approve the pull request. The misspelling of the variable cashe_result
should be corrected to cache_result
to ensure clarity and functionality. Additionally, the typo in the error message 'Value is muttable' should be corrected to 'Value is mutable'. Once these changes are made, your code will be much clearer and more effective. Keep up the great work, and I look forward to your updated submission! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review
button in the Reviewers tab.
app/main.py
Outdated
@@ -2,5 +2,22 @@ | |||
|
|||
|
|||
def cache(func: Callable) -> Callable: | |||
# Write your code here | |||
pass | |||
cashe_result ={} |
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 variable cashe_result
is misspelled. It should be cache_result
to correctly reflect its purpose as a cache storage.
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.
cashe_result ->cache_result
Done
app/main.py
Outdated
def func_results(*args, **kwargs): | ||
for arg in args: | ||
if not isinstance(arg,(int,str,float, tuple)): | ||
print("Value is muttable") |
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 message 'Value is muttable' contains a typo. It should be 'Value is mutable'.
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.
"Value is muttable" ->"Value is mutable"
Done
app/main.py
Outdated
return | ||
for keys, value in kwargs.items(): | ||
if not isinstance(value,(int,str,float, tuple)): | ||
print("Value is muttable") |
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 message 'Value is muttable' contains a typo. It should be 'Value is mutable'.
all problems are solved |
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.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
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.
all problems are solved
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.
Great job on implementing the caching decorator function! 🎉 You've successfully addressed previous issues, such as correcting the spelling of cache_result
and fixing the error message to "Value is mutable". The decorator now correctly checks for immutable arguments and handles caching efficiently.
However, there are a couple of points to improve:
- Ensure the error message is consistently corrected to "Value is mutable".
- Update the function signature to return a
Callable
type instead ofNone
to align with the expected return type of the decorator.
These improvements will enhance the functionality and correctness of your code. Keep up the good work, and remember to address these issues in future projects to avoid similar problems. Your dedication to learning and improving is commendable! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
pass | ||
cache_result = {} | ||
|
||
def func_results(*args, **kwargs) -> 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 return type in the function signature should be Callable
instead of None
to match the expected return type of the decorator.
def func_results(*args, **kwargs) -> None: | ||
for arg in args: | ||
if not isinstance(arg, (int, str, float, tuple)): | ||
print("Value is mutable") |
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 error message 'Value is mutable' should be corrected to 'Value is mutable' to fix the typo.
return | ||
for keys, value in kwargs.items(): | ||
if not isinstance(value, (int, str, float, tuple)): | ||
print("Value is mutable") |
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 error message 'Value is mutable' should be corrected to 'Value is mutable' to fix the typo.
No description provided.