-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
MAINT Parameters validation for affinity_propagation #24868
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
Conversation
Thanks, @Ala-Na. While the implementation is correct, I check, and Therefore, additional input validations were avoided in the function, to optimize for execution time. So I propose that we remove the validation. However, I think it is worth updating the docstring to make it more obvious to our users. So in the
You can put this note at the end of the "Notes" section. |
@glemaitre should we make function to call the class instead, like dbscan? |
Indeed, I think we should strive for that. We would only pass I feel that it tends to be overengineered to avoid some parameter initialization redundancies. |
OK so I looked a bit more into the problem here. I open #24884 which would be the best solution. Indeed, when a class is called in the function, we delegate the parameter validation of the function to the class. We already have this pattern in several places and we have only to check that all parameters of the function will be validated by the class. In @Ala-Na I will close this PR but in case, you want to take another function, you can check which pattern is used and in case it requires a refactoring, I can help at guiding how to do it. |
Reference Issues/PRs
Towards #24862
What does this implement/fix? Explain your changes.
Added
@validate_params decorator
for affinity_propagation public function.Any other comments?
According to @glemaitre this PR may not be useful as a
_parameters_constraints
is present in theAffinityPropagation
class (present in same file, the class calls the public function).He still advised me to create corresponding pull request as it may need some deeper look to know if it could be of use or not.
First open source contribution, please let me know how I can improve this work =)