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

Paper - Brittney Payne #56

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

Conversation

pollipayne
Copy link

No description provided.

Copy link
Collaborator

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

Outstanding work Brittney. This is a great submission. I made a few suggestions, but all the code here works and works well. Awesome job!

# test 2 if title returns none function returns none
# test 3 if genre returns none function returns none
# test 4 if rating returns none function returns none
def create_movie(movie_title, genre, rating):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍


# test 5 funct to add movie to "watched" list, takes in
# user data (list of dict) and a movie dict returns new list
def add_to_watched(user_data, movie):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍


# test 6 func to add movie to watchlist, takes in user data (list of dict)
# and a movie dict and returns the new list
def add_to_watchlist(user_data, movie):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

# test 8 moves a movie from watched list to watchlist when watchlist is truthy

# test 9 function does nothing if movie title is not in watched list
def watch_movie(user_data, title):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

# test 10 funct for avg rating that takes in user_data returns avg of ratings

# # test 11 function returns 0.0 if no movies in watched list
def get_watched_avg_rating(user_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 , nice use of a list comprehension!

# watched all the movies friends have watched.


def get_friends_unique_watched(user_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

# c: the host of the movie is in the users "subscriptions"

# test 20 ensure you return an empty list if there are no intersections
def get_available_recs(user_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 , This has a pretty deep list comprehension. It's good practice, but be a little careful if the logic is getting too twisted for readability's sake.

Comment on lines +175 to +176
most_watched_genre = get_most_watched_genre(user_data)
friends_watched_user_has_not = get_friends_unique_watched(user_data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good reuse of functions.

# movies in watch list


def get_new_rec_by_genre(user_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

# of recco movies that should be added to the list only if in users
# favorites and if none of the friends have watched.

def get_rec_from_favorites(user_data):
Copy link
Collaborator

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