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 - reyna diaz #147

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

Tigers - reyna diaz #147

wants to merge 6 commits into from

Conversation

reynamdiaz
Copy link

it's time to party

new_movie["genre"] = genre
new_movie["rating"] = rating
return new_movie

Choose a reason for hiding this comment

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

Well done! Just to condense this code slightly, you could just create the movie dictionary within the if statement or even simply return it as a dictionary:

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

user_data["watched"].append(movie)

return user_data

Choose a reason for hiding this comment

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

Awesome! These extra line breaks aren't necessary for a function this small, but they won't cause any issues.

Copy link

@apradoada apradoada left a comment

Choose a reason for hiding this comment

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

Overall, great job, Reyna! As mentioned throughout the comments, there are a few places where the spacing could be a little more consistent and a few places where you could have condensed some code. Other than that, well done! GREEN!

def add_to_watchlist(user_data, movie):
user_data["watchlist"].append(movie)

return user_data

Choose a reason for hiding this comment

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

👍

for movie in user_data["watchlist"]:
if movie["title"] == title:
user_data["watchlist"].remove(movie)
user_data["watched"].append(movie)

Choose a reason for hiding this comment

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

An alternative here to help you get in the practice of using helper methods would be to use the add_to_watched function you created before!

add_to_watched(user_data, movie)

if movie["title"] == title:
user_data["watchlist"].remove(movie)
user_data["watched"].append(movie)

Choose a reason for hiding this comment

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

You could also put a second return statement here within the if statement. That will allow the for loop to break as soon as it finds the movie it's looking for. This isn't crucial, but it can definitely save some time/space if our data set is large.

rating_average += movie["rating"]

return rating_average / len(user_data["watched"])

Choose a reason for hiding this comment

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

Just a few notes here. Notice that you do use user_data["watched"] quite a bit here. To save us some trouble, we could simply make a variable that holds that list and use it instead. Also, as a general rule, we don't need to add in the line break right after an if statement definition just because it makes it easier to see what code is part of the if block (Line 41).

popular_genre[genre] = 1
else:
popular_genre[genre] += 1

Choose a reason for hiding this comment

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

We could split these into two separate functions. One that finds the frequency of each genre and one that finds the one that appears most frequently!

@@ -118,13 +118,13 @@ def test_moves_movie_from_watchlist_to_empty_watched():
# Assert
assert len(updated_data["watchlist"]) == 0
assert len(updated_data["watched"]) == 1

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

Choose a reason for hiding this comment

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

Typically, we'll place the value we are testing on the left and the constant on the right!


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

Choose a reason for hiding this comment

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

Just to be safe, we may want to test our "genre" and "rating" at the same time to make sure the entire movie was brought in correctly!

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

@pytest.mark.skip()
assert movie_to_watch in updated_data["watched"]

Choose a reason for hiding this comment

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

👍

assert FANTASY_4 in friends_unique_movies
assert HORROR_1 in friends_unique_movies
assert INTRIGUE_3 in friends_unique_movies
# @pytest.mark.skip()

Choose a reason for hiding this comment

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

👍

recommendations = get_new_rec_by_genre(sonyas_data)

# Assert
assert recommendations == []

Choose a reason for hiding this comment

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

👍

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.

2 participants