DOC Ensures that sklearn.utils.validation.has_fit_parameter passes numpydoc validation #21471
Conversation
| @@ -231,7 +231,7 @@ | |||
| "sklearn.utils.validation.check_memory", | |||
| "sklearn.utils.validation.check_random_state", | |||
| "sklearn.utils.validation.column_or_1d", | |||
| "sklearn.utils.validation.has_fit_parameter", | |||
| # "sklearn.utils.validation.has_fit_parameter", | |||
There was a problem hiding this comment.
It seems that you changed more than just this function. You might want to uncomment all the functions that you modified to be sure that there is not other problem
There was a problem hiding this comment.
@glemaitre I thought that when I changed this file, it was only related to one function. But, now I see it's all those functions in one file. But, I didn't run the pytest on those other functions, I only fixed putting backticks on the parameters.
Also, sometimes I don't know where the error is (what line number), so I believe what I do is make the change in a function until the error goes away.
I can work on the other 6 functions within validation, but in separate PRs, because I just ran pytest against the others and there are still errors which need to be fixed separate for each of them.
There was a problem hiding this comment.
OK fine with me. The thing is that you could have delayed the change with the backtick for the other PRs as well. Like this, the scope of the changes is really limited to a single place.
There was a problem hiding this comment.
I will keep that in mind for the future. :)
There was a problem hiding this comment.
To avoid introducing confusing unnecessary merge conflicts with other concurrent PRs by first time contributors also working on #21350, I would rather have PRs only focused on a specific function at a time.
There was a problem hiding this comment.
@reshamas I guess what @ogrisel is asking, is to revert the changes which are not related to the function you're directly addressing in this PR, and to open a separate PR for them, which I agree with, especially if it helps with fewer merge conflicts with other PRs.
Could you elaborate what the output you see is, so that we could maybe help you find the relevant places to fix?
…_has_fit_parameter
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
…scikit-learn into np_v_has_fit_parameter
…_has_fit_parameter
|
@ogrisel @glemaitre |
…_has_fit_parameter
Reference Issues/PRs
Addresses #21350
What does this implement/fix? Explain your changes.
NumPy errors + formatting
Any other comments?
#DataUmbrella sprint