-
Notifications
You must be signed in to change notification settings - Fork 8
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
Started basis examples #253
Conversation
Some tests are failing and I will fix them tomorrow. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
=======================================
Coverage 97.09% 97.09%
=======================================
Files 20 20
Lines 1857 1857
=======================================
Hits 1803 1803
Misses 54 54 ☔ View full report in Codecov by Sentry. |
Fixed comments from PR: #251 Added examples to |
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.
A few small changes and this is also good to go. Thanks!
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.
Variety of changes requested, but the same themes repeated.
In particular, I think we shouldn't show the to_transformer method in the example of the basis init method. It's not the "standard use case" for the basis object, which is what those should be showing
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.
Remove the init of additive basis from the example, and this is ready to merge too.
I merge development also, I have merged the GLM example PR and I am going to merge another PR as well before this one (the SVRG auto-stepsize)
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.
Once you make the change Edoardo mentioned, this is good with me!
I deleted the old glm redundant branch and created a new one. I included the comments Edoardo made on the previous basis PR. But I need to add more examples for the subclasses and inherited methods and double check that all docstring results are correct. Hope everything is clean now!
Old basis branch PR: #251