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

Recommendation engine class #66

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Conversation

cocomittens
Copy link
Collaborator

Ok this is currently untested & still working on it, but I believe at least the converting ids to titles functionalities should theoretically work? Just putting this up here to show progress.

for title in titles:
netflix_id = self.get_movie_id(title)
titles.append(netflix_id)
return netflix_ids
Copy link
Collaborator

@jhanley634 jhanley634 Jan 25, 2025

Choose a reason for hiding this comment

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

FYI, if we call this approach (A.), there's a pair of identical ways to accomplish the same thing.

(B.)

    def titles_to_ids(self, titles):
        return [self.get_movie_id(title) for title in titles]

(C.)

    def titles_to_ids(self, titles):
        return list(map(self.get_movie_id, titles))

Is one of these "better"? No, not exactly. During early development and debugging we might prefer (A.), as we can insert debug print()'s, breakpoint(), and so on. Later on a list comprehension might make more sense. I personally prefer (C.) because it highlights how very very simple the processing is -- we're just applying a function, nothing else, no tricks up my sleeves, it's just one function. But YMMV.

Notice that generators like map() and range() are "lazy", so we use list() to pull all the results out of the generator. Consider range(2) versus list(range(2)).

(Please advance this out of "draft" status once the code is ready for merging to main.)

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