Closed mvargas33 closed 2 years ago
@mvargas33 We should avoid breaking compatibility right away: instead, you should keep older names with 'deprecated' value and have deprecation warnings for the next release (0.7.0), before we remove them permanently in 0.8.0.
See for instance what was done here: #193
@bellet
So, the existing code is not broken anymore. Following #193 I've added a warning mentioning deprecation in each part of the code. These warnings are FutureWarnings
in order to show them to end users. If I use DeprecationWarning
the warning won't show up unless the user configure their program to show more warnings (according to docs and tested myself).
To take note:
check_estimator
test from sklearn, checks that all params received in the constructor are set in the Estimator
, including the deprecated one, even if it is not used anywhere.Changes look good, merging.
Closes #257 as suggested by @bellet . Tests pass as expected.
This refactor is relevant from a user perspective.