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

Cedar - Maria Obregon Garcia #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mobregong
Copy link

No description provided.

@mobregong mobregong closed this Sep 20, 2021
@mobregong mobregong reopened this Sep 20, 2021
Copy link

@kaidamasaki kaidamasaki left a comment

Choose a reason for hiding this comment

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

Sorry for the late feedback. Getting sick last week put me behind schedule with grading.

Great job!

There were a few little things but overall everything looked good!

Comment on lines +59 to +72
if user_data["watched"]:
# populate watched_genres freq map
for movie in user_data["watched"]:
genre = movie["genre"]
if genre in watched_genres:
watched_genres[genre] += 1
else:
watched_genres[genre] = 1
# find max frequency and paired genre
max_watched_genre_freq = max(watched_genres.values())
for genre, freq in watched_genres.items():
if freq == max_watched_genre_freq:
most_watched_genre = genre
return most_watched_genre

Choose a reason for hiding this comment

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

You can clean this up a little bit with an early return (personally I find extra indentation makes functions a little harder to read):

Suggested change
if user_data["watched"]:
# populate watched_genres freq map
for movie in user_data["watched"]:
genre = movie["genre"]
if genre in watched_genres:
watched_genres[genre] += 1
else:
watched_genres[genre] = 1
# find max frequency and paired genre
max_watched_genre_freq = max(watched_genres.values())
for genre, freq in watched_genres.items():
if freq == max_watched_genre_freq:
most_watched_genre = genre
return most_watched_genre
if not user_data["watched"]:
return None
# populate watched_genres freq map
for movie in user_data["watched"]:
genre = movie["genre"]
if genre in watched_genres:
watched_genres[genre] += 1
else:
watched_genres[genre] = 1
# find max frequency and paired genre
max_watched_genre_freq = max(watched_genres.values())
for genre, freq in watched_genres.items():
if freq == max_watched_genre_freq:
most_watched_genre = genre
return most_watched_genre

Comment on lines +77 to +90
# non-assigned helper function
def create_user_watched_set(user_data):
user_watched_set = set()
for movie in user_data["watched"]:
user_watched_set.add(movie["title"])
return user_watched_set

# non-assigned helper function
def create_friends_watched_set(user_data):
friends_watched_set = set()
for friend in user_data["friends"]:
for movie in friend["watched"]:
friends_watched_set.add(movie["title"])
return friends_watched_set

Choose a reason for hiding this comment

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

Nice use of helper functions!

Comment on lines +93 to +97
if not movie["title"] in user_watched_set:
movie_not_in_user_watched = True
else:
movie_not_in_user_watched = False
return movie_not_in_user_watched

Choose a reason for hiding this comment

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

You can condense this down because in produces a boolean:

Suggested change
if not movie["title"] in user_watched_set:
movie_not_in_user_watched = True
else:
movie_not_in_user_watched = False
return movie_not_in_user_watched
return not movie["title"] in user_watched_set

recommended_movies = []
user_watched_set = create_user_watched_set(user_data)
movie_rec = {}
already_recommended = []

Choose a reason for hiding this comment

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

If you make already_recommended a set then this is more efficient.

Comment on lines +112 to +113
movie_rec = {"title": movie["title"], "host": movie["host"]}
recommended_movies.append(movie_rec)

Choose a reason for hiding this comment

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

You can use the original movie here instead of creating a new one:

Suggested change
movie_rec = {"title": movie["title"], "host": movie["host"]}
recommended_movies.append(movie_rec)
recommended_movies.append(movie)

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.

2 participants