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.

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

Tigers - Paje Bishop #158

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 13 additions & 17 deletions tests/test_wave_01.py
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()

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()

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()

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()

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()

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()

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()

def test_moves_movie_from_watchlist_to_empty_watched():
# Arrange
janes_data = {
Expand All @@ -119,12 +119,10 @@ 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.")
# *******************************************************************************************
# ****** Add assertions here to test that the correct movie was added to "watched" **********
# *******************************************************************************************
assert janes_data["watched"][0]["genre"] == GENRE_1

Choose a reason for hiding this comment

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

This assertion is fine as a quick check, but definitely keep in mind that you're not limited to one assert here, and being through in your testing can have benefits. For example, I would at least add assertions to check that the "title" and "rating" keys are correct too.




@pytest.mark.skip()
def test_moves_movie_from_watchlist_to_watched():
# Arrange
movie_to_watch = HORROR_1
Expand All @@ -143,12 +141,10 @@ 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.")
# *******************************************************************************************
# ****** Add assertions here to test that the correct movie was added to "watched" **********
# *******************************************************************************************
assert updated_data["watched"][1] == HORROR_1

Choose a reason for hiding this comment

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

Comparatively, this assertion is better because we're comparing the whole dictionary in updated_data["watched"][1] to HORROR_1, so all keys will be compared.

I personally would also check that the right movie was removed from updated_data["watchlist"], but as our instructions don't mention that, not really a miss on your part.




@pytest.mark.skip()
def test_does_nothing_if_movie_not_in_watchlist():
# Arrange
movie_to_watch = HORROR_1
Expand Down
8 changes: 4 additions & 4 deletions tests/test_wave_02.py
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()

def test_calculates_watched_average_rating():
# Arrange
janes_data = clean_wave_2_data()
Expand All @@ -14,7 +14,7 @@ def test_calculates_watched_average_rating():
assert average == pytest.approx(3.58333)
assert janes_data == clean_wave_2_data()

@pytest.mark.skip()

def test_empty_watched_average_rating_is_zero():
# Arrange
janes_data = {
Expand All @@ -27,7 +27,7 @@ def test_empty_watched_average_rating_is_zero():
# Assert
assert average == pytest.approx(0.0)

@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()

def test_genre_is_None_if_empty_watched():
# Arrange
janes_data = {
Expand Down
15 changes: 6 additions & 9 deletions tests/test_wave_03.py
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()

def test_my_unique_movies():
# Arrange
amandas_data = clean_wave_3_data()
Expand All @@ -16,7 +16,7 @@ def test_my_unique_movies():
assert INTRIGUE_2 in amandas_unique_movies
assert amandas_data == clean_wave_3_data()

@pytest.mark.skip()

def test_my_not_unique_movies():
# Arrange
amandas_data = clean_wave_3_data()
Expand All @@ -28,7 +28,7 @@ def test_my_not_unique_movies():
# Assert
assert len(amandas_unique_movies) == 0

@pytest.mark.skip()

def test_friends_unique_movies():
# Arrange
amandas_data = clean_wave_3_data()
Expand All @@ -43,7 +43,7 @@ def test_friends_unique_movies():
assert FANTASY_4 in friends_unique_movies
assert amandas_data == clean_wave_3_data()

@pytest.mark.skip()

def test_friends_unique_movies_not_duplicated():
# Arrange
amandas_data = clean_wave_3_data()
Expand All @@ -54,13 +54,10 @@ def test_friends_unique_movies_not_duplicated():

# Assert
assert len(friends_unique_movies) == 3

assert friends_unique_movies.count(INTRIGUE_3) == 1

Choose a reason for hiding this comment

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

Interestingly enough, I actually think this assertion here is fine even though it doesn't check every movie in the list. The main goal of this test is to check that movies don't get duplicated more than the entire list is correct as per our instructions.

Good test design!


raise Exception("Test needs to be completed.")
# *************************************************************************************************
# ****** Add assertions here to test that the correct movies are in friends_unique_movies **********
# **************************************************************************************************

@pytest.mark.skip()
def test_friends_not_unique_movies():
# Arrange
amandas_data = {
Expand Down
6 changes: 3 additions & 3 deletions tests/test_wave_04.py
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()

def test_get_available_friend_rec():
# Arrange
amandas_data = clean_wave_4_data()
Expand All @@ -16,7 +16,7 @@ def test_get_available_friend_rec():
assert FANTASY_4b in recommendations
assert amandas_data == clean_wave_4_data()

@pytest.mark.skip()

def test_no_available_friend_recs():
# Arrange
amandas_data = {
Expand All @@ -38,7 +38,7 @@ def test_no_available_friend_recs():
# Assert
assert len(recommendations) == 0

@pytest.mark.skip()

def test_no_available_friend_recs_watched_all():
# Arrange
amandas_data = {
Expand Down
21 changes: 11 additions & 10 deletions tests/test_wave_05.py
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()

def test_new_genre_rec():
# Arrange
sonyas_data = clean_wave_5_data()
Expand All @@ -17,7 +17,7 @@ def test_new_genre_rec():
assert FANTASY_4b in recommendations
assert sonyas_data == clean_wave_5_data()

@pytest.mark.skip()

def test_new_genre_rec_from_empty_watched():
# Arrange
sonyas_data = {
Expand All @@ -38,7 +38,7 @@ def test_new_genre_rec_from_empty_watched():
# Assert
assert len(recommendations) == 0

@pytest.mark.skip()

def test_new_genre_rec_from_empty_friends():
# Arrange
sonyas_data = {
Expand All @@ -52,13 +52,14 @@ def test_new_genre_rec_from_empty_friends():
}
]
}
# Act
recommendations = get_new_rec_by_genre(sonyas_data)

# Assert
assert recommendations == None


raise Exception("Test needs to be completed.")
# *********************************************************************
# ****** Complete the Act and Assert Portions of theis tests **********
# *********************************************************************

@pytest.mark.skip()
def test_unique_rec_from_favorites():
# Arrange
sonyas_data = clean_wave_5_data()
Expand All @@ -72,7 +73,7 @@ def test_unique_rec_from_favorites():
assert INTRIGUE_2b in recommendations
assert sonyas_data == clean_wave_5_data()

@pytest.mark.skip()

def test_unique_from_empty_favorites():
# Arrange
sonyas_data = {
Expand All @@ -94,7 +95,7 @@ def test_unique_from_empty_favorites():
# Assert
assert len(recommendations) == 0

@pytest.mark.skip()

def test_new_rec_from_empty_friends():
# Arrange
sonyas_data = {
Expand Down
94 changes: 88 additions & 6 deletions viewing_party/party.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,105 @@
# ------------- WAVE 1 --------------------

def create_movie(title, genre, rating):
pass
if title and genre and rating:
movie = {"title": title, "genre": genre, "rating": rating}
else:
movie = None
return movie

def add_to_watched(user_data, movie):
user_data["watched"] = [movie]
return user_data

Choose a reason for hiding this comment

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

Ah, this is kind of a mistake on our tests but there is a bug here that they don't catch.

Line 9 here will replace the entire value of user_data["watched"]. That's fine if there are no entries in the list, but if there had been entries in the list, we'd lose them in favor of the new list with one entry.

Not too major and sorry for our tests missing it. That being said, usually the expected behavior of adding to a list is to append the new element.

Choose a reason for hiding this comment

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

Plus you otherwise seem to know how to use append (for example in the below function) so not major enough an issue to possibly move down to Yellow or Green.


def add_to_watchlist(user_data, movie):
user_data["watchlist"] = [movie]
return user_data

Choose a reason for hiding this comment

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

The same bug as add_to_watched applies here too. But as our tests didn't catch it, that's on us more than you.


def watch_movie(user_data, movie_title):
for movies in user_data["watchlist"]:
if movies["title"] == movie_title:
user_data["watched"].append(movies)
user_data["watchlist"].remove(movies)
return user_data

# -----------------------------------------
# ------------- WAVE 2 --------------------
# -----------------------------------------
def get_watched_avg_rating(user_data):
ratings_list = []
for movies in user_data["watched"]:
ratings_list.append(movies["rating"])
if ratings_list:
rating_average = sum(ratings_list)/len(ratings_list)
else:
rating_average = 0.0
return rating_average


def get_most_watched_genre(user_data):
genres = []
for movies in user_data["watched"]:
genres.append(movies["genre"])
if genres:
most_watched_genre = max(genres, key = genres.count)

Choose a reason for hiding this comment

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

This works, but there's a minor performance hit. max is an O(n) function and so is the count method. In this case max will end up calling count on every element in genres, bringing the whole complexity up to O(n^2). But there are ways to solve this in O(n) time, for example, using a frequency dictionary.

Now, as we hadn't even covered time complexity when you wrote this, this is not a major enough problem that I would move away from a Green. But I am pointing it out!

else:
most_watched_genre = None
return most_watched_genre
# -----------------------------------------
# ------------- WAVE 3 --------------------
# -----------------------------------------
def get_unique_watched(user_data):
unique_watched = []
friends_watched = []
for i in range(len(user_data["friends"])):

Choose a reason for hiding this comment

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

Nothing wrong with using range here, but this could read a little cleaner not using it:

for friend in user_data["friends"]:
    for item in friend["watched"]:
        friends_watched.append(item)

Mostly just a minor style nitpick, though. The same goes for a few other loops here and in get_friends_unique_watched but I won't belabor the point.

for item in user_data["friends"][i]["watched"]:
friends_watched.append(item)
for movie in user_data["watched"]:
if movie not in friends_watched:
unique_watched.append(movie)
return unique_watched

def get_friends_unique_watched(user_data):
friends_watched = []
friends_unique_watched = []
for i in range(len(user_data["friends"])):
for item in user_data["friends"][i]["watched"]:
friends_watched.append(item)
for movie in friends_watched:
if (movie not in user_data["watched"]
and movie not in friends_unique_watched):
friends_unique_watched.append(movie)

return friends_unique_watched


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

def get_available_recs(user_data):
friends_unique_watched = get_friends_unique_watched(user_data)
reccommendations = []
for movie in friends_unique_watched:
if movie["host"] in user_data["subscriptions"]:
reccommendations.append(movie)
print(reccommendations)

Choose a reason for hiding this comment

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

I'm guessing this print statement was for debugging purposes. Not major, but usually good style to make sure to remove these after you're done debugging.

return reccommendations
# -----------------------------------------
# ------------- WAVE 5 --------------------
# -----------------------------------------
def get_new_rec_by_genre(user_data):
favorite_genre = get_most_watched_genre(user_data)
friends_movies = get_friends_unique_watched(user_data)
rec_list_by_genre = []
if friends_movies:
for movie in friends_movies:
if movie["genre"] == favorite_genre:
rec_list_by_genre.append(movie)
else:
return None

Choose a reason for hiding this comment

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

Looking at our instructions, I'm reading them as we were expecting that an empty list would be returned in this case, but it's completely legitimate that one would return None too!

However not really a fault on your part because it seems we didn't test this case in our tests either!

return rec_list_by_genre

def get_rec_from_favorites(user_data):
user_unique_movies = get_unique_watched(user_data)
rec_list_from_favorites = []
for movie in user_data["watched"]:
if (movie in user_unique_movies
and movie in user_data["favorites"]):
rec_list_from_favorites.append(movie)
return rec_list_from_favorites