-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
MAINT validate parameters in NaiveBayes estimators #23674
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
MAINT validate parameters in NaiveBayes estimators #23674
Conversation
The original docstring states, that alpha=0 will result in no smoothing. But the Estimators using alpha as a parameter are based on |
I don't understand this mypy error: https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=43423&view=logs&j=32e2e1bb-a28f-5b18-6cfc-3f01273f5609&t=f370d024-dfa6-536d-2b2d-766ea2a2900c&l=14 When I run |
This behavior is also addressed in #22269 and is expected to change soon(ish). I think, for this PR, it would probably be best for you to preserve existing behavior, i.e. allow a user to enter |
Alright, I am going to reverse my changes regarded _ALPHA_MIN. I'll probably get to it tomorrow :) |
I revert |
Oh, I missed that. I checked the PR section, but I filtered for the ticket number, since the issue told us to put the ticket number in the title. At least we had the same solution :D |
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 push a small refactoring for alpha
and made a proper inheritance for the constraints that are more intuitive.
Ah, that's a good idea. If the constraints of the child class equal the parent class, just use the parent class. |
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. I just pushed a small change to only validate at the first call to partial_fit
Thanks @Diadochokinetic |
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com> Co-authored-by: jeremiedbb <jeremiedbb@yahoo.fr>
Reference Issues/PRs
Make all estimators use _validate_params #23462
What does this implement/fix? Explain your changes.
Add parameter_constraints to the classes in sklearn.naive_bayes.py, add validate_params in fit function, remove redundant parameter tests and correct documentation.