Phoenix-Jenny Massari-Anh Tran #16
Conversation
…xt Learn purposes
There was a problem hiding this comment.
🟢 GREEN! 🟢
Well done, y'all! Your code passes all the tests and looks good! I've left some suggestions, but nothing major that requires a second pass! That being said, you are more than welcome to make changes if you so desire! I think overall, the big things I saw were just finding a balance of the docstrings and figuring out what assumptions can be made to cut down on certain code! Other than that it looks great! Well done!
| assert updated_data["watched"] == [{ | ||
| "title": MOVIE_TITLE_1, | ||
| "genre": GENRE_1, | ||
| "rating": RATING_1 | ||
| }] | ||
|
|
There was a problem hiding this comment.
I like this test! One small change I would suggest might be to make the left side a little more specific! If we're testing to see if an entire list looks like what we're expecting, this is ok, but what we're most curious about is if the movie has been added. I would suggest checking something like:
assert updated_data["watched"][0] == {
"title": MOVIE_TITLE_1,
"genre": GENRE_1,
"rating": RATING_1
}
| # ******************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movie was added to "watched" ********** | ||
| # ******************************************************************************************* | ||
| assert updated_data["watched"] == [FANTASY_2, HORROR_1] |
There was a problem hiding this comment.
I like what y'all have done here! Y'all know exactly what the "watched" list should look like after the movie gets moved!
| # ************************************************************************************************* | ||
| # ****** Add assertions here to test that the correct movies are in friends_unique_movies ********** | ||
| # ************************************************************************************************** | ||
|
|
||
| @pytest.mark.skip() | ||
| assert friends_unique_movies == [FANTASY_4, HORROR_1, INTRIGUE_3] |
There was a problem hiding this comment.
Like the other assert you created, I love that you leveraged the fact that you know exactly what the list should look like!
| empty_list = get_new_rec_by_genre(sonyas_data) | ||
|
|
||
| # Assert | ||
| assert empty_list == [] |
There was a problem hiding this comment.
This act and assert are exactly what we were looking for!
| """ | ||
|
|
||
| if not title or not genre or rating is None: | ||
| return None |
| if movie not in user_watched and movie not in unique_friends_watched: | ||
| unique_friends_watched.append(movie) | ||
|
|
||
| return unique_friends_watched |
There was a problem hiding this comment.
This looks good! I love that you used your helper function in multiple functions! Really really well done!
| # using max() to get the max count in genere_count dictionary | ||
| # most_frequent_genre = max(genres_count, key=genres_count.get) | ||
| # If use max(dictionary), max() looks only at the keys of the dictionary, not the values. | ||
| # If the keys are strings, max() will return the key that comes last alphabetically |
There was a problem hiding this comment.
This feels like it was a comment meant for the developers eyes only! Don't forget to take them out!
|
|
||
| recommended_movies = [] | ||
| for movie in friends_watched: | ||
| if movie not in user_watched and movie["host"] in user_data["subscriptions"] \ |
There was a problem hiding this comment.
Remember that get_friends_watched is going to hold all the movies your friends have seen and you haven't! Therefore, we don't actually need to check and make sure there is no overlap. Also, because recommended_movies starts empty, we have other checks that make sure duplicates aren't added. As a result, the first and last conditionals aren't necessary!
| for movie in friends_watched: | ||
| if movie not in user_watched and \ | ||
| movie["genre"] == most_frequenty_genre and\ | ||
| movie not in recommended_movies: |
There was a problem hiding this comment.
Same idea here as before. Every other collection creation should check for duplicates, which means this conditional should be unnecessary!
| Returns: | ||
| recomemded movies (list) | ||
| """ | ||
| user_watched = user_data["watched"] |
There was a problem hiding this comment.
We only use user_watched one other time in this function! When we use a value (`user_data["watched"]') just a few times, it can be a good idea to just avoid using a variable as a middle man. Direct access will work just fine!
No description provided.