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 #726

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

solution #726

wants to merge 2 commits into from

Conversation

Familenko
Copy link

No description provided.

app/main.py Outdated

@wraps(func)
def wrapper(*args) -> Any:
nonlocal cached, arg_cache

Choose a reason for hiding this comment

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

this statement is redundant, as dict is mutable type

Copy link
Author

Choose a reason for hiding this comment

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

I do not use dict

Choose a reason for hiding this comment

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

Lists are also mutable, so mind this comment

app/main.py Outdated

return result

else:

Choose a reason for hiding this comment

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

You can use a single return statement out of the if/else statement instead of two inside it.

Choose a reason for hiding this comment

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

You missed this one

app/main.py Outdated


def cache(func: Callable) -> Callable:
# Write your code here
pass
cached = []

Choose a reason for hiding this comment

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

your approach is incorrect: you should store arguments and the result of the function with these arguments and return this result without calling function when arguments are the same.
hint: use dictionary as storage

Copy link
Author

Choose a reason for hiding this comment

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

I want to use list becouse it is more comfortable to operate list as a row in DataFrame in case if it will be reciered to store as structured info. And both of the list have same input index for ich time use. I saw that a lot of people use dic in this task, but my way also give same output and additionaly more comfortable for Data Scierntist job (in case if it will be required to analize cash)

Choose a reason for hiding this comment

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

You should use a dict here. Under the hood dicts in python use hash tables. You will go through this in Dict Advanced topic but shortly: it works faster than list. You will retrieve values with much less efforts
Using a dictionary provides a more concise and clear representation of the data
It's sutable if you will have a dict instead of two lists for Data Analysis. Data scientists often work with dictionaries or other data types, different from the python lists (numpy arrays for example)

Copy link
Author

Choose a reason for hiding this comment

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

I used this decorator when train a machine learning model. During the experiment I needed to go through different variants of hyperparameters and correlate these values with the values already obtained. In case of failure to find the optimal parameters, I could use the saved cache values to return to the place where I had started on a non-straight path. In this case, the order of the experiments was also important to me (which could be taken into account in the cache only when using a list)

The task conditions do not say what exactly the caching will be used for, if for the log, then you are right and the speed of information retrieval matters and you should use a dictionary. But if the task is similar to the one I performed with this decorator, then the order of caching is more important than the fact that the result will be 0.0001 seconds slower.

So, since the priority of performance or usability was not explicitly stated, why is my option so unpleasant? And as you say "Data scientists often work with numpy arrays» wich is vary easy to get from Python list, but not dict

app/main.py Outdated

return result

else:

Choose a reason for hiding this comment

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

You missed this one

app/main.py Outdated


def cache(func: Callable) -> Callable:
# Write your code here
pass
cached = []

Choose a reason for hiding this comment

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

You should use a dict here. Under the hood dicts in python use hash tables. You will go through this in Dict Advanced topic but shortly: it works faster than list. You will retrieve values with much less efforts
Using a dictionary provides a more concise and clear representation of the data
It's sutable if you will have a dict instead of two lists for Data Analysis. Data scientists often work with dictionaries or other data types, different from the python lists (numpy arrays for example)

app/main.py Outdated

@wraps(func)
def wrapper(*args) -> Any:
nonlocal cached, arg_cache

Choose a reason for hiding this comment

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

Lists are also mutable, so mind this comment

Copy link

@fsocie7y fsocie7y left a comment

Choose a reason for hiding this comment

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

well done 👍

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.

4 participants