Skip to content

Zoisite D19 - Yvett J Viewing Party#44

Open
yvett-codes wants to merge 12 commits into
Ada-C19:mainfrom
yvett-codes:main
Open

Zoisite D19 - Yvett J Viewing Party#44
yvett-codes wants to merge 12 commits into
Ada-C19:mainfrom
yvett-codes:main

Conversation

@yvett-codes

Copy link
Copy Markdown

No description provided.

@yangashley yangashley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice job on Viewing Party, Whitney and Yvett!

Your project shows that you both are getting more fluent with working with nested data structures, iterating over them, and using conditional logic!

Nice job making frequent commits!

Comment thread tests/test_wave_01.py
# Assert
assert len(updated_data["watchlist"]) == 0
assert len(updated_data["watched"]) == 1
assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Consider also confirming the genre and rating are as expected.

Comment thread tests/test_wave_01.py
assert len(updated_data["watched"]) == 2

raise Exception("Test needs to be completed.")
assert updated_data["watched"][1]["title"] == movie_to_watch["title"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remember that we can use == to compare the keys and values in a dictionary all at once, so here we could

assert updated_data["watched"][1] == HORROR_1

We could also check that the movie we expect to be left in the watchlist list is still there, and that the other watched movie is properly in the watched list.

Comment thread tests/test_wave_03.py
assert len(friends_unique_movies) == 3

raise Exception("Test needs to be completed.")
assert INTRIGUE_3 in friends_unique_movies

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Line 56 has a check that the length of friends_unique_movies == 3. It would make your test more robust if we could add two more assertions to confirm that the other two movies in the list are the ones that we expect them to be.

assert HORROR_1 in friends_unique_movies
assert FANTASY_4 in friends_unique_movies

Comment thread tests/test_wave_05.py

raise Exception("Test needs to be completed.")
# Assert
assert len(recommendations) == 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread viewing_party/party.py
Comment on lines +1 to +2
import statistics
from statistics import mode

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can get away with deleting line 1 and also importing mean on line 2:

from statistics import mode, mean

If you do that then you can update line 43. Line 43, you wrote statistics.mean(ratings), but on line 49 you wrote mode(genres).

If you update your imports then line 43 can look like 49 and become mean(ratings).

Comment thread viewing_party/party.py
friends_watched = get_friends_unique_watched(user_data)
for movie in friends_watched:
if movie not in user_data["watched"]:
if movie["host"] in user_data["subscriptions"] and movie not in recommended:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think you can also drop the second part of the check here and movie not in recommended because get_friends_unique_watched already checks that you don't append friends' movies more than once.

On line 65, you have if movie not in user_movies and movie not in unique_movies: so you won't have duplicates in friends_watched that could get added more than once to recommended.

Also we could improve the subscription lookup if we trade a little space and make a set of the subscriptions (which are strings, so can go in a set). Using in on a list is a linear operation, but on a set, it's constant.

subscription_lookup = set(user_data["subscriptions"]) 
for movie in friends_watched: 
    if movie["host"] in subscription_lookup: 
        recommended.append(movie)

Comment thread viewing_party/party.py

def get_new_rec_by_genre(user_data):
recommended = []
if not len(user_data["watched"]):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can drop len() here

Comment thread viewing_party/party.py
Comment on lines +92 to +93
genres = [movie["genre"] for movie in user_data["watched"]]
genre = mode(genres)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Notice that these same lines of code are repeated from lines 48 and 49. Could we reuse get_most_watched_genre that you've already written?

Also, the variable genre on line 93 is a particular kind of genre. Consider renaming it so it's more descriptive - like most_watched_genre

Comment thread viewing_party/party.py

genres = [movie["genre"] for movie in user_data["watched"]]
genre = mode(genres)
friends_watched = get_friends_unique_watched(user_data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Prefer variable name to include datatype

Comment thread viewing_party/party.py
genre = mode(genres)
friends_watched = get_friends_unique_watched(user_data)
for movie in friends_watched:
if movie["genre"] == genre and movie not in user_data["watched"] and movie not in recommended:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

get_friends_unique_watched returns a list of movies that friends have watched but the user has not watched so we can drop the check movie not in user_data["watched"] here. Also if friends_watched doesn't have any duplicate movies in it, could we assume that the same movie won't get appended to recommended more than once? If we can make that assumption, then we could drop the check for movie not in recommended too.

I noticed that line 96 is longer than what Python style guide recommends. If you were to have all 3 checks on line 96, consider storing the return value of the expressions in descriptively named variable to help shorten this line

is_not_in_user_data = movie not in user_data["watched"]
is_not_in_recommended = movie not in recommended
if movie["genre"] == genre and is_not_in_user_data and is_not_in_recommended:

It also increases readability.

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.

3 participants