-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: add support for creating a Matrix Factorization model #1330
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Tim Sweña (Swast) <swast@google.com>
Co-authored-by: Tim Sweña (Swast) <swast@google.com>
Co-authored-by: Tim Sweña (Swast) <swast@google.com>
5f4f9d3
to
e5165a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to add some system/unit tests for this change?
…is/python-bigquery-dataframes into b338873783-matrix-factorization
user_col: str, | ||
item_col: str, | ||
rating_col: str = "rating", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GarrettWu @shuoweil I see in #1282 you ended up passing in "id_col" as a separate argument to fit()
instead of the class constructor. Is this a pattern you would recommend here?
Note: MatrixFactorization differs somewhat from that application in that normally in scikit-learn one would have a "sparse matrix" data type (e.g. https://docs.scipy.org/doc/scipy/reference/sparse.html) where rows/cols/values would all be bundled up in one object, similar to how we are using the bigframes DataFrame for this purpose.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕