Snow Leopards- Ja Hopkins #153

jahopkins · 2022-09-23T15:50:22Z

I couldn't finish wave 3

…er to return 0.0 rating if the watched list is empty

anselrognlie

This is a good start.

The tests for the waves you completed are passing, and that code is well-organized. I included feedback on the first function of wave 3, as well as an example of applying the suggested changes to that function.

Take a look at that, and then let's get together to look over the rest of the project!

anselrognlie · 2022-09-24T22:34:30Z

tests/test_wave_01.py

-    raise Exception("Test needs to be completed.")
+    # raise Exception("Test needs to be completed.")
+
+    assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1


anselrognlie · 2022-09-24T22:36:23Z

tests/test_wave_01.py

+    assert updated_data["watched"][1] == movie_to_watch
+    assert movie_to_watch in updated_data["watched"]


These two checks are more or less doing the same thing. Consider using three in checks, one for checking which movie is remaining int the watchlist list, and two for each of the movies we expect to find in the watched list

anselrognlie · 2022-09-24T22:45:23Z

tests/test_wave_02.py

@@ -27,7 +27,7 @@ def test_empty_watched_average_rating_is_zero():
    # Assert


👀 It looks like you missed uncommenting the skip on the test on line 18. When I did so, the test passed. 👍

anselrognlie · 2022-09-24T22:57:02Z

viewing_party/party.py

+    # dictionary contains 3 key-value pairs
+    # return none if falsy
+    title_genre_rating = {}
+    if title and genre and rating:


Nice truthy checks. But consider reversing the sense of the comparison so that the more error-like condition is handled within the conditional check, and the main logic (making the dictionary) doesn't need to be nested.

Structuring the checks to look for the more error-like conditions is a pattern referred to as a guard clause, and it's a common way of organizing our code.

if not title or not genre or not rating: # code to run when one of the inputs is falsy # code to run when all inputs are truthy

anselrognlie · 2022-09-24T22:59:04Z

viewing_party/party.py

+    # return none if falsy
+    title_genre_rating = {}
+    if title and genre and rating:
+        title_genre_rating.update({'title': title, 'genre': genre, 'rating': rating})


Notice that the argument to update is a newly created dictionary. We could assign that new dictionary directly to our variable, or even return it, rather than updating the empty dictionary we started with.

title_genre_rating = {'title': title, 'genre': genre, 'rating': rating}

anselrognlie · 2022-09-25T00:28:24Z

viewing_party/party.py

+            genres_watched[movie["genre"]] += 1
+
+    num_of_genres_watched = genres_watched.values()
+    max_num_genres_watched = max(num_of_genres_watched)


Nice use of max to find the highest count, but notice then that we must iterate back through the data again and to find a genre that had this count. Alternatively, we could iterate over the frequency map ourselves to find the max count, keeping track of the key (genre) ourself.

We'll see that there's really not a performance difference, though, and arguably this code is a little simpler. (but writing a max finder ourself is great practice!).

anselrognlie · 2022-09-25T00:44:17Z

viewing_party/party.py

+    unique_movies = []
+    for watched_movie in user_data["watched"]:
+    # friends_movies.append(watched_movie)
+        for movie in user_data["friends"]:


A more accurate name for what this variable represents is friend. We're iterating over the friends list, which is a list of friend dictionaries, not movie dictionaries.

anselrognlie · 2022-09-25T00:45:47Z

viewing_party/party.py

+    # friends_movies.append(watched_movie)
+        for movie in user_data["friends"]:
+    #     for movie in user_data["friends"]:
+            if watched_movie["title"] in unique_movies:


The function is supposed to be building up a list of movies not their titles, so I would expect unique_movies to contain movies, not titles.

anselrognlie · 2022-09-25T00:48:05Z

viewing_party/party.py

+    #     for movie in user_data["friends"]:
+            if watched_movie["title"] in unique_movies:
+                continue
+            elif watched_movie["title"] not in movie["watched"]:


From above, movie would be more accurately called friend. So this code is checking whether the title (a string) of the movie we're looking at is in a list of movies (each is a dictionary), which will never be true. A string does not equal a dictionary (python will not look inside the dictionaries for matching values).

So to determine whether a the watched_movie is something this friend has watched, we should check something like

elif watched_movie not in friend["watched"]:

anselrognlie · 2022-09-25T00:56:12Z

viewing_party/party.py

+            if watched_movie["title"] in unique_movies:
+                continue
+            elif watched_movie["title"] not in movie["watched"]:
+                unique_movies.append(watched_movie["title"])


Even if we make the above change, we can't be certain that watched_movie should be added to the reasult list. It hasn't been watched by this friend, but it could still have been watched by a different friend.

What we could do is use a boolean value to see whether we've seen the movie in any of the friend's lists (initialized to False, since we wouldn't have found the movie yet). If we find the movie, set the flag to True. If we get through all the friends and haven't found the movie, then we can add it to the result list.

Try adding this additional logic yourself. I have one possible way of doing this in the next comment.

unique_movies = [] for watched_movie in user_data["watched"]: found_movie = False for friend in user_data["friends"]: if watched_movie in unique_movies: found_movie = True elif watched_movie in friend["watched"]: found_movie = True if not found_movie: unique_movies.append(watched_movie) return unique_movies

…oid duplication

…ue movies watched by friends

…friends is FANTASY_1 movie

anselrognlie

Looks good!

I added a few more comments primarily around how we could think about improving the time complexity in Wave 3.

anselrognlie · 2022-10-24T17:50:51Z

tests/test_wave_03.py

+    assert friends_unique_movies[-2] == HORROR_1
+    assert friends_unique_movies[0] == FANTASY_4


There weren't any particular specifications as to the ordering of the movies in the list of results. As a result, I would tend to use an in check for the movies we expect to see, since that won't require knowing at what exact position each movie ended up. This lets us enforce the required behaviors in the test without being overly strict.

In fact, the checks from the previous test are exctly what we would need to add.

assert INTRIGUE_3 in friends_unique_movies assert HORROR_1 in friends_unique_movies assert FANTASY_4 in friends_unique_movies

Since this test already has a check for the list having 3 items, if we then check for 3 expected items and find them all, this also means that each item appears in the result only once (no duplication).

anselrognlie · 2022-10-24T17:58:53Z

viewing_party/party.py

+    for friend in user_data["friends"]:
+        for movie_details in friend["watched"]:
+            movie_title = movie_details["title"]
+            movies_watched_by_friends.append(movie_title)


Nice way to build up a list of movie titles the friends have watched. Notice that since you decided to use the title as representative of the entire movie record ( areasonable decision given our data), we could use a set for the movies_watched_by_friends collection rather than a list. This would improve the complexity of the in check on line 102 (since sets have constant time lookup, while a list is linear time).

anselrognlie · 2022-10-24T18:00:51Z

viewing_party/party.py

+        if movie_watched not in movies_watched_by_friends:
+            unique_movies.append(movie)
+            # watched_titles.append(movie["title"])
+    return(unique_movies)


No need for () around the return value. We can wrap the value with () if we are returning the result of a long expression and need to line wrap, but otherwise, we generally leave them off. We would also leave a space between return and the () around the expression, since return isn't a function and we don't want it to look like we're doing a function call.

anselrognlie · 2022-10-24T18:02:12Z

viewing_party/party.py

+    for movie in user_data["watched"]:
+        movie_watched = movie["title"]
+        # add all the movies watched to new list
+        all_movies_watched.append(movie_watched)


As above, since you're using the title as a proxy for the entire moview, we could use a set for all_movies_watched for improved in check performance.

anselrognlie · 2022-10-24T18:04:51Z

viewing_party/party.py

+        for movie_details in friend["watched"]:
+            movie_title = movie_details["title"]
+            if (movie_title not in all_movies_watched and 
+                movie_details not in unique_movies_watched_by_friends):


Nice way to filter out a movie that's already been added from another friend. Here, we do need to build a list of movie detail records, not just the title. So unique_movies_watched_by_friends must be a list. However, if we still wanted to gain the constant in lookup even for the result list, we could create another set in which we store only the titles of the movies we've added to unique_movies_watched_by_friends, and look up a movie's title in that set before adding it to the result list, and adding the title to the set if we do end up adding a movie to the result list.

As it is, doing the in checks (which are linear) in a loop (essentially linear) gives us an overall quadratic time complexity. But we could get this down to linear by building that extra set, since the operations in the loop would become constant.

anselrognlie · 2022-10-24T18:09:01Z

viewing_party/party.py


 # -----------------------------------------
 # ------------- WAVE 4 --------------------
 # -----------------------------------------

+def get_available_recs(user_data):
+    recommended_movies = []
+    friends_watched = get_friends_unique_watched(user_data)


👍 Great reuse of your get_friends_unique_watched function to simplify the implementation here.

lavazane added 8 commits September 15, 2022 15:01

changed pass to return home

54cc945

define create_movie function

82d1f18

Made assertions for Wave 1 tests

35bbe89

defined get_watched_avg_rating function

cd13032

updated get_watched_avg_rating function by adding if statement in ord…

aba741b

…er to return 0.0 rating if the watched list is empty

defined function named ge_most_watched_genre

9bbee29

added an if statement to return None if the watched list is empty

03ee31c

final commit in order to push project

b96d781

anselrognlie reviewed Sep 25, 2022

View reviewed changes

lavazane added 7 commits October 14, 2022 03:38

Define get_unique_watched function

8ac67e6

defined get_friends_unique_watched function

dee698f

add conditional statement to check if movie_details are in list to av…

714724e

…oid duplication

add assertions to test in order to avoid duplicate movies in the uniq…

18a56f1

…ue movies watched by friends

Add assertion to check if the first item in unique movies watched by …

3ae983f

…friends is FANTASY_1 movie

define get_available_recs functions

fa912bc

commit changes, before pushing. Updated the test waves

c2f0102

anselrognlie reviewed Oct 24, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snow Leopards- Ja Hopkins #153

Snow Leopards- Ja Hopkins #153

jahopkins commented Sep 23, 2022

anselrognlie left a comment

anselrognlie Sep 24, 2022

anselrognlie Sep 24, 2022

anselrognlie Sep 24, 2022

anselrognlie Sep 24, 2022

anselrognlie Sep 24, 2022

anselrognlie Sep 25, 2022

anselrognlie Sep 25, 2022

anselrognlie Sep 25, 2022

anselrognlie Sep 25, 2022

anselrognlie Sep 25, 2022

anselrognlie Sep 25, 2022

anselrognlie left a comment

anselrognlie Oct 24, 2022

anselrognlie Oct 24, 2022

anselrognlie Oct 24, 2022

anselrognlie Oct 24, 2022

anselrognlie Oct 24, 2022

anselrognlie Oct 24, 2022

		assert updated_data["watched"][1] == movie_to_watch
		assert movie_to_watch in updated_data["watched"]

		@@ -27,7 +27,7 @@ def test_empty_watched_average_rating_is_zero():
		# Assert

		assert friends_unique_movies[-2] == HORROR_1
		assert friends_unique_movies[0] == FANTASY_4

Snow Leopards- Ja Hopkins #153

Are you sure you want to change the base?

Snow Leopards- Ja Hopkins #153

Conversation

jahopkins commented Sep 23, 2022

anselrognlie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anselrognlie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment