Kunzite - Danqing Liu Viewing Party #54
Conversation
There was a problem hiding this comment.
Great work on this! I left some specific feedback on relevant lines. Generally, I found there to be a good use of variable names and helper functions as well as some syntax not yet covered in the material. I'd advise caution with one specific use of a list comprehension (see line 95 in party.py) and take care with writing tests so your assertions test the right cases. Again, great job overall!
| raise Exception("Test needs to be completed.") | ||
| # raise Exception("Test needs to be completed.") | ||
|
|
||
| assert MOVIE_TITLE_1 in updated_data["watched"][0]["title"] |
There was a problem hiding this comment.
This check definitely works because it checks for a substring in a string and they happen to be identical, i.e. it does
"It Came from the Stack Trace" in "It Came from the Stack Trace"However, it would also pass if the movie title were:
"It Came from the Stack Trace" in "It Came from the Stack Trace AND OTHER STUFF TOO"which isn't exactly the test you want. So it would in this case be better to test for string equality, like
MOVIE_TITLE_1 == updated_data["watched"][0]["title"]| # raise Exception("Test needs to be completed.") | ||
|
|
||
|
|
||
| assert movie_to_watch in updated_data["watched"] |
There was a problem hiding this comment.
Good job with these, these correctly verify that the movie is in the updated watched list and not in the updated watchlist!
| @@ -54,13 +54,17 @@ def test_friends_unique_movies_not_duplicated(): | |||
|
|
|||
| # Assert | |||
| assert len(friends_unique_movies) == 3 | |||
| assert INTRIGUE_3 in friends_unique_movies | |||
There was a problem hiding this comment.
Nice job with this set of assertions!
| recommendations = get_new_rec_by_genre(sonyas_data) | ||
|
|
||
|
|
||
| assert len(recommendations) == 0 |
| movie["genre"] = genre | ||
| movie ["rating"] = rating | ||
| return movie | ||
| else: |
There was a problem hiding this comment.
Since you return from the if clause, there's no need to use an else. You could simply return None on the same line of indentation as the main body of the function.
| # user_unique_movies.append(movie) | ||
| # return(user_unique_movies) | ||
|
|
||
| user_unique_movie = [movie for movie in user_data["watched"] if movie not in get_friends_movies(user_data)] |
| # friends_unique_movies.append(movie) | ||
| friends_unique_movies =[movie for movie in get_friends_movies(user_data) if movie not in user_data["watched"]] | ||
|
|
||
| [friends_result.append(movie) for movie in friends_unique_movies if movie not in friends_result] |
There was a problem hiding this comment.
This does work to append movies to the friends_result list, but generally list comprehensions should be used to return a new list, rather than to mutate another list while executing the comprehension.
| # ----------------------------------------- | ||
| # ------------- WAVE 4 -------------------- | ||
| # ----------------------------------------- | ||
| def get_available_recs(user_data): |
| recommended_movies = [] | ||
| unique_watched_friend_movies = get_friends_unique_watched(user_data) | ||
|
|
||
| if genre is None or unique_watched_friend_movies ==[]: |
There was a problem hiding this comment.
Good job checking whether genre or unique_watched_friend_movies are None and empty list respectively. You can use not here again to simplify the expressions:
if not genre or not unique_watched_friend_movies:
return recommended_movies # here since you defined recommended_movies, no need to create another empty list just to return it| recommended_movies.append(movie) | ||
| return recommended_movies | ||
|
|
||
| def get_rec_from_favorites(user_data): |
There was a problem hiding this comment.
Excellent job with this one as well 🎉
No description provided.