-
Notifications
You must be signed in to change notification settings - Fork 328
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
Implements linear_model.LinearRegression
#2260
Conversation
Thanks for your contribution, could you add licenses at the head of new files? |
Sure |
Hi @hekaisheng, so far I have added license for new files and replaced np operators with mt operators. The |
Great! I will take a look at it in this week. |
Have passed all tests except |
Hi @hekaisheng, can I extend job timeout threshold to something longer, like 150min?, https://github.com/mars-project/mars/pull/2260/checks?check_run_id=3279790160 |
The test job for learn costs less than 60 minutes on master, it seems unreasonable that your cases need more than one hour |
Oh sorry I didnt see your msg |
It runs for less then 20mins in my local laptop, not sure what's going on here, lets just re-run the tests |
It's better to use the fixture
|
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.
I left some comments and please add some cases to increase the coverage.
ecac73a
to
dc1201b
Compare
Hi @hekaisheng, it seems all good 😇. Anything else to refine? Also, do I need to open another non-draft PR or you can directly merge this one? |
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.
LGTM
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.
LGTM
Co-authored-by: Fernando <42331572+Fernadoo@users.noreply.github.com>
What do these changes do?
Related issue number