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.

Sign up for GitHub

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

Snow Leopards- Ja Hopkins #153

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Snow Leopards- Ja Hopkins #153

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
54cc945
changed pass to return home
lavazane Sep 15, 2022
82d1f18
define create_movie function
lavazane Sep 21, 2022
35bbe89
Made assertions for Wave 1 tests
lavazane Sep 23, 2022
cd13032
defined get_watched_avg_rating function
lavazane Sep 23, 2022
aba741b
updated get_watched_avg_rating function by adding if statement in ord…
lavazane Sep 23, 2022
9bbee29
defined function named ge_most_watched_genre
lavazane Sep 23, 2022
03ee31c
added an if statement to return None if the watched list is empty
lavazane Sep 23, 2022
b96d781
final commit in order to push project
lavazane Sep 23, 2022
8ac67e6
Define get_unique_watched function
lavazane Oct 14, 2022
dee698f
defined get_friends_unique_watched function
lavazane Oct 14, 2022
714724e
add conditional statement to check if movie_details are in list to av…
lavazane Oct 14, 2022
18a56f1
add assertions to test in order to avoid duplicate movies in the uniq…
lavazane Oct 17, 2022
3ae983f
Add assertion to check if the first item in unique movies watched by …
lavazane Oct 17, 2022
fa912bc
define get_available_recs functions
lavazane Oct 17, 2022
c2f0102
commit changes, before pushing. Updated the test waves
lavazane Oct 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions tests/test_wave_01.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from viewing_party.party import *
from tests.test_constants import *

@pytest.mark.skip()
# @pytest.mark.skip()
def test_create_successful_movie():
# Arrange
movie_title = MOVIE_TITLE_1
Expand All @@ -19,7 +19,7 @@ def test_create_successful_movie():
assert new_movie["genre"] == GENRE_1
assert new_movie["rating"] == pytest.approx(RATING_1)

@pytest.mark.skip()
# @pytest.mark.skip()
def test_create_no_title_movie():
# Arrange
movie_title = None
Expand All @@ -32,7 +32,7 @@ def test_create_no_title_movie():
# Assert
assert new_movie is None

@pytest.mark.skip()
# @pytest.mark.skip()
def test_create_no_genre_movie():
# Arrange
movie_title = "Title A"
Expand All @@ -45,7 +45,7 @@ def test_create_no_genre_movie():
# Assert
assert new_movie is None

@pytest.mark.skip()
# @pytest.mark.skip()
def test_create_no_rating_movie():
# Arrange
movie_title = "Title A"
Expand All @@ -58,7 +58,7 @@ def test_create_no_rating_movie():
# Assert
assert new_movie is None

@pytest.mark.skip()
# @pytest.mark.skip()
def test_adds_movie_to_user_watched():
# Arrange
movie = {
Expand All @@ -79,7 +79,7 @@ def test_adds_movie_to_user_watched():
assert updated_data["watched"][0]["genre"] == GENRE_1
assert updated_data["watched"][0]["rating"] == RATING_1

@pytest.mark.skip()
# @pytest.mark.skip()
def test_adds_movie_to_user_watchlist():
# Arrange
movie = {
Expand All @@ -100,7 +100,7 @@ def test_adds_movie_to_user_watchlist():
assert updated_data["watchlist"][0]["genre"] == GENRE_1
assert updated_data["watchlist"][0]["rating"] == RATING_1

@pytest.mark.skip()
# @pytest.mark.skip()
def test_moves_movie_from_watchlist_to_empty_watched():
# Arrange
janes_data = {
Expand All @@ -119,12 +119,16 @@ def test_moves_movie_from_watchlist_to_empty_watched():
assert len(updated_data["watchlist"]) == 0
assert len(updated_data["watched"]) == 1

raise Exception("Test needs to be completed.")
# raise Exception("Test needs to be completed.")

assert updated_data["watched"][0]["title"] == MOVIE_TITLE_1
Copy link

Choose a reason for hiding this comment

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

👍

assert updated_data["watched"][0]["genre"] == GENRE_1
assert updated_data["watched"][0]["rating"] == RATING_1
# *******************************************************************************************
# ****** Add assertions here to test that the correct movie was added to "watched" **********
# *******************************************************************************************

@pytest.mark.skip()
# @pytest.mark.skip()
def test_moves_movie_from_watchlist_to_watched():
# Arrange
movie_to_watch = HORROR_1
Expand All @@ -143,12 +147,15 @@ def test_moves_movie_from_watchlist_to_watched():
assert len(updated_data["watchlist"]) == 1
assert len(updated_data["watched"]) == 2

raise Exception("Test needs to be completed.")
# raise Exception("Test needs to be completed.")

assert updated_data["watched"][1] == movie_to_watch
assert movie_to_watch in updated_data["watched"]
Comment on lines +152 to +153
Copy link

Choose a reason for hiding this comment

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

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

# *******************************************************************************************
# ****** Add assertions here to test that the correct movie was added to "watched" **********
# *******************************************************************************************

@pytest.mark.skip()
# @pytest.mark.skip()
def test_does_nothing_if_movie_not_in_watchlist():
# Arrange
movie_to_watch = HORROR_1
Expand Down
6 changes: 3 additions & 3 deletions tests/test_wave_02.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from viewing_party.party import *
from tests.test_constants import *

@pytest.mark.skip()
# @pytest.mark.skip()
def test_calculates_watched_average_rating():
# Arrange
janes_data = clean_wave_2_data()
Expand All @@ -27,7 +27,7 @@ def test_empty_watched_average_rating_is_zero():
# Assert
Copy link

Choose a reason for hiding this comment

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

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

assert average == pytest.approx(0.0)

@pytest.mark.skip()
# @pytest.mark.skip()
def test_most_watched_genre():
# Arrange
janes_data = clean_wave_2_data()
Expand All @@ -39,7 +39,7 @@ def test_most_watched_genre():
assert popular_genre == "Fantasy"
assert janes_data == clean_wave_2_data()

@pytest.mark.skip()
# @pytest.mark.skip()
def test_genre_is_None_if_empty_watched():
# Arrange
janes_data = {
Expand Down
2 changes: 1 addition & 1 deletion tests/test_wave_03.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from viewing_party.party import *
from tests.test_constants import *

@pytest.mark.skip()
# @pytest.mark.skip()
def test_my_unique_movies():
# Arrange
amandas_data = clean_wave_3_data()
Expand Down
93 changes: 90 additions & 3 deletions viewing_party/party.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,110 @@
# ------------- WAVE 1 --------------------


def create_movie(title, genre, rating):
pass
# if truthy return a dictionary
# dictionary contains 3 key-value pairs
# return none if falsy
title_genre_rating = {}
if title and genre and rating:
Copy link

Choose a reason for hiding this comment

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

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

title_genre_rating.update({'title': title, 'genre': genre, 'rating': rating})
Copy link

Choose a reason for hiding this comment

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

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}

return(title_genre_rating)
Copy link

Choose a reason for hiding this comment

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

return isn't a function, so we don't need the parentheses around the value we're returning. In python, the only reason we'd put parentheses is if we were returning the result of a complex expression and wanted to be able to implicitly wrap the line. Notice we would also put a space after the return to keep it from looking as much like a function call.

    return ("this" + "is" + "a" + "long" + "expression"
        + "that" + "I" + "want" + "to" + "wrap")

return None
Copy link

Choose a reason for hiding this comment

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

Taking all that feedback, we might end up with a function like this:

    if not title or not genre or not rating:
        return None

    return {'title': title, 'genre': genre, 'rating': rating}


def add_to_watched(user_data, movie):
Copy link

Choose a reason for hiding this comment

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

👍

user_data["watched"].append(movie)
return user_data

# user_data = {watched: [{movie}{moveie}]}
# add each movie to the watched key

def add_to_watchlist(user_data, movie):
Copy link

Choose a reason for hiding this comment

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

👍

# user_data = {watchlist: [{movies}{user}]{wants}{to_watch}}
# add movie into the watchlist inside of user data
# return user_data
user_data["watchlist"].append(movie)
return(user_data)
Copy link

Choose a reason for hiding this comment

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

As above, no need for the parens on the return here (or throughout the rest of the functions).


def watch_movie(user_data, title):
# user_data = {watchlist: watched:}
# if title is in watch_list then add to watched:
# return user_data
# if not in watch list:
# return user_data

for movie in user_data["watchlist"]:
if title in movie.values():
Copy link

Choose a reason for hiding this comment

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

This could potentially find the title under some other key than the expected "title" key. We know what key we're interested in checking, so we should check it directly.

        if title == movie["title"]:

user_data["watchlist"].remove(movie)
user_data["watched"].append(movie)
Copy link

Choose a reason for hiding this comment

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

Consider breaking out of the looping after performing the remove. Remember that modifying a loop wihle iterating over it (the watchlist list here) can result in unexpected behavior. It's probably not a problem here, since we only expect to find one of the movie in the list, but we can make that explicit by using break to terminate the iteration as soon as we modify the list.

Also, a blank line after this would help visually separate the return (which is outside the loop) from the loop body.

return(user_data)



# -----------------------------------------
# ------------- WAVE 2 --------------------
# -----------------------------------------

def get_watched_avg_rating(user_data):
# user data = {watched: [{movie2}, {movie2}]}
user_rating = []

if len(user_data["watched"]) == 0:
Copy link

Choose a reason for hiding this comment

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

Nice handling for the case of an empty list.

A slightly more pythonic way to check for whether a list is empty is

    if not some_list_var:
        # code to handle empty list

So here,

    if not user_data["watched"]:

average_rating = 0.0
return average_rating
else:
Copy link

Choose a reason for hiding this comment

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

Since the if condition results in a return (effectively a guard clause), we can leave the else off, and unindent the following code. That helps emphasize that it's the more important code.

for movie in user_data["watched"]:
user_rating.append(movie["rating"])
Comment on lines +55 to +56
Copy link

Choose a reason for hiding this comment

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

We haven't talked about list comprehensions in python, but there's a special syntax that lets us effectively append a set of values very concisely.

The following would initialize user_rating to be a list containing the ratings of each movie from the watched list.

    user_rating = [movie["rating"] for movie in user_data["watched"]]

The general pattern lets us take some code like

new_list = []
for item in old_list:
    if should_process(item):
        new_list.append(process(item))

and write it like:

new_list = [process(item) for item in old_list if should_process(item)]

average_rating = (sum(user_rating)) / (len(user_rating))
Copy link

Choose a reason for hiding this comment

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

Notice that since this is indented under the loop, we're recalculating the average each time through the loop. We only need to do this once (after the loop), after we've added all of the ratings to the list in the loop.

# print(user_rating)
# print(average_rating)
return(average_rating)

def get_most_watched_genre(user_data):

genres_watched = {}

if not user_data["watched"]:
Copy link

Choose a reason for hiding this comment

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

👍 Nice and Pythonic!

return None

for movie in user_data["watched"]:
if movie["genre"] not in genres_watched:
genres_watched[movie["genre"]] = 1
elif movie["genre"] in genres_watched:
Copy link

Choose a reason for hiding this comment

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

A key is either in a dict or it's not. So we can use a else: without an expression, rather than an elif:.

genres_watched[movie["genre"]] += 1

num_of_genres_watched = genres_watched.values()
max_num_genres_watched = max(num_of_genres_watched)
Copy link

Choose a reason for hiding this comment

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

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!).


for genre, times_watched in genres_watched.items():
if times_watched == max_num_genres_watched:
return genre



# -----------------------------------------
# ------------- WAVE 3 --------------------
# -----------------------------------------

def get_unique_watched(user_data):
# user_data = {watched :[{"title: title"}, {"title": title}], friends: [{watched_key:[{"title:title"}{title:title}]}{watched_key:[{title:title}{title:title}]}]}
# each movie dict in watched_key has a title

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

Choose a reason for hiding this comment

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

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.

# for movie in user_data["friends"]:
if watched_movie["title"] in unique_movies:
Copy link

Choose a reason for hiding this comment

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

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.

continue
elif watched_movie["title"] not in movie["watched"]:
Copy link

Choose a reason for hiding this comment

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

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"]:

unique_movies.append(watched_movie["title"])
Copy link

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

    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

return unique_movies


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

# -----------------------------------------
# ------------- WAVE 5 --------------------
# -----------------------------------------

# -----------------------------------------