MAINT rename base_estimator to estimator in RANSACRegressor #22062
Conversation
|
Thanks @trujillo9616 for your pull request. |
There was a problem hiding this comment.
Otherwise, it looks good. I did not check yet the reason for the CI to fail.
Please add an entry to the change log at doc/whats_new/v1.1.rst. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:.
| @@ -65,7 +65,7 @@ class RANSACRegressor( | |||
|
|
|||
| Parameters | |||
| ---------- | |||
| base_estimator : object, default=None | |||
| estimator : object, default=None | |||
There was a problem hiding this comment.
we still need the base_estimator docstring
base_estimator : object, default="deprecated"
Use `estimator` instead.
.. deprecated:: 1.1
`base_estimator` is deprecated and will be removed in 1.3.
Use `estimator` instead.
|
So the CI is failing because you did not document |
|
@cmarmo @glemaitre Thank you for your feedback and code review! I've updated the docstring in the RANSACRegressor class and the |
doc/whats_new/v1.1.rst
Outdated
| - |Enhancement| Rename parameter `base_estimator` to `estimator` in | ||
| :class:`linear_model.RANSACRegressor` to improve readability and consistency. | ||
| `base_estimator` is deprecated and will be removed in 1.3. | ||
| :pr:`22062` by :user:`Adrian Trujillo <trujillo9616>`. |
There was a problem hiding this comment.
Please group the |Enhancement| entries together, before the |Fix| entries.
There was a problem hiding this comment.
Sure, l'll update the changelog in a bit. Thanks for the review!
|
Hello @trujillo9616 , thank you for your work. Some conflicts arose. Do you mind fixing them in synchronizing with main? You might want to have a look to the github documentation for that. |
|
@cmarmo I resolved the conflicts, thanks for the guidance! All the tests have passed now 🥳 , I'll be on the lookout in case you suggest further modifications. |
|
Thanks @trujillo9616, all green again... now we just wait for someone pressing the merge button! :) |
Reference Issues/PRs
See #9104. This PR enhances some of the proposed changes in the referenced issue.
What does this implement/fix? Explain your changes.
For RANSACRegressor:
This enhancement renames the
base_estimatorparameter toestimatorin the RANSACRegressor class for improved readability and consistency.For
test_ransac.py:Tests in the
test_ransac.py. file were modified for using the updated parameter. A test case was added to check if the warning has been raised when setting thebase_estimatorparameter.