Skip to content
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

Solution #1495

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Solution #1495

wants to merge 2 commits into from

Conversation

rkostiv253
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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 this task! 😊 While your code is on the right track, there are a few critical issues that need to be addressed to ensure the caching mechanism functions correctly:

  1. Unhashable Return Type: The long_time_func_2 function returns a list, which is unhashable and cannot be used as a dictionary key. Consider converting the list to a tuple or another hashable type if you need to cache the result.

  2. Hashable Arguments: Ensure that the args used as keys in the cache dictionary are hashable. Unhashable types will raise an error, so you might need to adjust how arguments are handled.

  3. Cache Structure: The cache store is currently initialized with func as a key, which is not ideal. Consider using a nested dictionary or a different structure to handle caching per function more effectively.

  4. Decorator Definition: The cache function is defined with func: tuple, which is incorrect. It should be func without specifying a type, as it is expected to be a function, not a tuple.

Once these issues are resolved, your code will be much more robust and meet the task requirements. Keep up the great work, and don't hesitate to reach out if you need further clarification or assistance! 🚀

✨ 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
def cache(func: Callable) -> Callable:
# Write your code here
pass
def cache(func: tuple) -> dict:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache function is defined with func: tuple, which is incorrect. It should be func without specifying a type, as it is expected to be a function, not a tuple.

Comment on lines +9 to +10
if func not in cache_store:
cache_store[func] = {}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache store is being initialized with func as a key, which is not ideal. Instead, consider using a nested dictionary or a different structure to handle caching per function.

if func not in cache_store:
cache_store[func] = {}

if args in cache_store[func]:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The args are used as keys in the cache dictionary. Ensure that args are hashable, as unhashable types will raise an error.

app/main.py Outdated

@cache
def long_time_func_2(n_tuple: tuple, power: int) -> int:
return [number ** power for number in n_tuple]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The long_time_func_2 function returns a list, which is unhashable and cannot be used as a dictionary key. Consider converting the list to a tuple or another hashable type if you need to cache the result.

@rkostiv253 rkostiv253 marked this pull request as draft January 17, 2025 17:12
@rkostiv253 rkostiv253 marked this pull request as ready for review January 17, 2025 17:12
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a 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 submission! 🎉 Your code is on the right track, but there are a few critical issues that need attention to ensure the caching mechanism functions correctly:

  1. Return Type of long_time_func_2: The function currently returns a tuple, which is hashable and suitable for caching. However, the task requires it to return a list. Please adjust the return type to a list to meet the task requirements.

  2. Cache Key Handling: Ensure that the cache keys are correctly handled. The use of args as a dictionary key can cause issues if it contains unhashable types. Make sure all elements in args are hashable or convert them to a hashable type.

  3. Decorator Definition: The type hint -> dict for the cache function is misleading. Since cache is a decorator, it should return a function. Consider updating the type hint to reflect this.

These changes are crucial for the functionality of your code. Please review the comments and make the necessary adjustments to improve your code. Keep up the good work, and continue learning from these experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

def cache(func: Callable) -> Callable:
# Write your code here
pass
def cache(func) -> dict:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type hint -> dict for the cache function is misleading. The cache function is a decorator and should return a function, not a dictionary. Consider removing the type hint or changing it to indicate that a function is returned.

if func not in cache_store:
cache_store[func] = {}

if args in cache_store[func]:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using args as a dictionary key can cause issues if args contains unhashable types, such as lists. Ensure that all elements in args are hashable or convert them to a hashable type.


if args in cache_store[func]:
print("Getting from cache")
return cache_store[func][*args]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expression cache_store[func][*args] is incorrect. The unpacking operator * should not be used here. Use args directly as the key.

else:
print("Calculating new result")
result = func(*args)
cache_store[func][*args] = result

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to line 14, the expression cache_store[func][*args] = result is incorrect. Use args directly as the key without the unpacking operator.


@cache
def long_time_func_2(n_tuple: tuple, power: int) -> int:
return tuple(number ** power for number in n_tuple)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function long_time_func_2 returns a tuple, which is hashable. However, if the intention was to return a list, consider converting it to a tuple before returning to ensure it can be used as a dictionary key.

@rkostiv253
Copy link
Author

from functools import wraps

def cache(func) -> dict:
cache_store = {}

@wraps(func)
def wrapper(*args) -> dict:
    if func not in cache_store:
        cache_store[func] = {}

    if args in cache_store[func]:
        print("Getting from cache")
        return cache_store[func][*args]
    else:
        print("Calculating new result")
        result = func(*args)
        cache_store[func][*args] = result
        return result

return wrapper

@cache
def long_time_func(num1: int, num2: int, num3: int) -> int:
return (num1 ** num2 ** num3) % (num1 * num3)

@cache
def long_time_func_2(n_tuple: tuple, power: int) -> int:
return tuple(number ** power for number in n_tuple)

print(long_time_func(1, 2, 3))
print(long_time_func(1, 2, 3))
print(long_time_func_2((5, 6, 7), 2))
print(long_time_func(1, 2, 3))
print(long_time_func_2((5, 6, 7), 2))
print(long_time_func_2((5, 6, 7), 2))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants