Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

FIX signature of deprecated classes #30145

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

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

lorentzenchr
Copy link
Member

Reference Issues/PRs

None. Popped up in #29097.

What does this implement/fix? Explain your changes.

Our common tests fail if a whole class is decorated by deprecated because the signature of __new__ changes to the generic (*args, **kwargs).

Any other comments?

Copy link

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 33465b4. Link to the linter CI: here

@adrinjalali
Copy link
Member

What was exactly the failure here?

@lorentzenchr
Copy link
Member Author

check_do_not_raise_errors_in_init_or_set_params failed

Full traceback

_ test_estimators[PassiveAggressiveClassifier(max_iter=5)-check_do_not_raise_errors_in_init_or_set_params] _
[gw0] linux -- Python 3.12.7 /usr/share/miniconda/envs/testvenv/bin/python

estimator = PassiveAggressiveClassifier(max_iter=5)
check = functools.partial(<function check_do_not_raise_errors_in_init_or_set_params at 0x7ff7ff70aca0>, 'PassiveAggressiveClassifier')
request = <FixtureRequest for <Function test_estimators[PassiveAggressiveClassifier(max_iter=5)-check_do_not_raise_errors_in_init_or_set_params]>>

    @parametrize_with_checks(list(_tested_estimators()))
    def test_estimators(estimator, check, request):
        # Common tests for estimator instances
        with ignore_warnings(
            category=(FutureWarning, ConvergenceWarning, UserWarning, LinAlgWarning)
        ):
>           check(estimator)

check      = functools.partial(<function check_do_not_raise_errors_in_init_or_set_params at 0x7ff7ff70aca0>, 'PassiveAggressiveClassifier')
estimator  = PassiveAggressiveClassifier(max_iter=5)
request    = <FixtureRequest for <Function test_estimators[PassiveAggressiveClassifier(max_iter=5)-check_do_not_raise_errors_in_init_or_set_params]>>

../1/s/sklearn/tests/test_common.py:120: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'PassiveAggressiveClassifier'
estimator_orig = PassiveAggressiveClassifier(max_iter=5)

    def check_do_not_raise_errors_in_init_or_set_params(name, estimator_orig):
        """Check that init or set_param does not raise errors."""
        Estimator = type(estimator_orig)
        params = signature(Estimator).parameters
    
        smoke_test_values = [-1, 3.0, "helloworld", np.array([1.0, 4.0]), [1], {}, []]
        for value in smoke_test_values:
            new_params = {key: value for key in params}
    
            # Does not raise
>           est = Estimator(**new_params)
E           TypeError: PassiveAggressiveClassifier.__init__() got an unexpected keyword argument 'args'

Estimator  = <class 'sklearn.linear_model._passive_aggressive.PassiveAggressiveClassifier'>
estimator_orig = PassiveAggressiveClassifier(max_iter=5)
name       = 'PassiveAggressiveClassifier'
new_params = {'args': -1, 'kwargs': -1}
params     = mappingproxy(OrderedDict({'args': <Parameter "*args">, 'kwargs': <Parameter "**kwargs">}))
smoke_test_values = [-1, 3.0, 'helloworld', array([1., 4.]), [1], {}, ...]
value      = -1

@adrinjalali adrinjalali merged commit d083972 into scikit-learn:main Oct 25, 2024
35 of 37 checks passed
@lorentzenchr lorentzenchr deleted the deprecated branch October 25, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.