-
Notifications
You must be signed in to change notification settings - Fork 4
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
Minor MLJ docstring fixes #133
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #133 +/- ##
=======================================
Coverage 97.00% 97.00%
=======================================
Files 22 22
Lines 734 734
=======================================
Hits 712 712
Misses 22 22 ☔ View full report in Codecov by Sentry. |
@ablam you are right with the scytipe error. i guess i must change
in
right? afterall i don't see why it shouldn't work with |
@ablaom regarding μ₀ and P₀ , i can change the structs but i will have to keep them in the Laplacereduc.laplace objects like this one for regression but also for classification
is this enough? |
your input_scitype correction looks consistent with the doc-string now. as to whether re the notation change: my suggestion is to modify only the keyword constructor to look something like i am only making a suggestion, and I expect @pat-alt will want to chime in here before you proceed with a change. you may want to kick the can and raise in a new issue tagged "next breaking release" or something . |
ok the scitype isn't a problem, it's just another type of feature. I ill fix the input_scitype. Regarding the other problem, |
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've left the descriptions for input
X
unchanged, despite the incongruence withinput_scitype
, as noted here.