8000 KBinsDiscretizer : Support inverse_transform for encode other than ordinal · Issue #11489 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

KBinsDiscretizer : Support inverse_transform for encode other than ordinal #11489

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.

Sign up for GitHub

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

Jump to bottom
Closed
qinhanmin2014 opened this issue Jul 12, 2018 · 11 comments
Closed

KBinsDiscretizer : Support inverse_transform for encode other than ordinal #11489

qinhanmin2014 opened this issue Jul 12, 2018 · 11 comments
Labels
Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted

Comments

@qinhanmin2014
Copy link
Member

Currently, we only support encode='ordinal' in inverse_transform. Since we've supported inverse_transform in OnehotEncoder, it's natural to support inverse_transform for encode='onehot' and encode='onehot-dense'. Store the fitted encoder will be a reasonable solution here. (See #11467 (comment))
@qinhanmin2014 qinhanmin2014 added Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted labels Jul 12, 2018
@ggc87
Copy link
Contributor
ggc87 commented Jul 12, 2018

Hi, can I try this?

@qinhanmin2014
Copy link
Member Author

welcome to contribute :)

@ggc87
Copy link
Contributor
ggc87 commented Jul 12, 2018

I'm having a small problem with the test test_non_meta_estimators. It fail an assertion due to the fact that on the fit process I'm storing the OHE and therfore __dict__ change.
Can someone help me with this? Should I do it in another way?

    @pytest.mark.parametrize(
            "name, Estimator, check",
            _generate_checks_per_estimator(_yield_all_checks,
                                           _tested_non_meta_estimators()),
            ids=_rename_partial
    )
    def test_non_meta_estimators(name, Estimator, check):
        # Common tests for non-meta estimators
        with ignore_warnings(category=(DeprecationWarning, ConvergenceWarning,
                                       UserWarning, FutureWarning)):
            estimator = Estimator()
            set_checking_parameters(estimator)
>           check(name, estimator)

Estimator  = <class 'sklearn.preprocessing._discretization.KBinsDiscretizer'>
check      = <function check_dict_unchanged at 0x7f8287cf5620>
estimator  = KBinsDiscretizer(encode='onehot', n_bins=5, strategy='quantile')
name       = 'KBinsDiscretizer'

sklearn/tests/test_common.py:99: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
sklearn/utils/testing.py:326: in wrapper
    return fn(*args, **kwargs)
sklearn/utils/estimator_checks.py:632: in check_dict_unchanged
    'Estimator changes __dict__ during %s' % method)
/usr/lib/python3.6/unittest/case.py:1121: in assertDictEqual
    self.fail(self._formatMessage(msg, standardMsg))

@qinhanmin2014
Copy link
Member Author

It's not uncommon to set attributes in fit? I don't think it will cause a test failure. Maybe you can submit a PR for us to review.

ggc87 pushed a commit to ggc87/scikit-learn that referenced this issue Jul 13, 2018
@Giovanni-whichit
KBinsDiscretizer : inverse_transform for ohe encoder [See scikit-lear…
5a62da2 
…n#11489]

This PR introduce inverse_transform in KBinsDiscretizer for encoder
other than ordinal.
@ggc87 ggc87 mentioned this issue Jul 13, 2018
Merged
@divayjindal95
Copy link
divayjindal95 commented Jul 14, 2018 •
edited
Loading

Is the issue still open ? Like can i try this?

@qinhanmin2014
Copy link
Member Author

@divayjindal95 @ggc87 has submitted a PR. Please try other issues without an active PR.

@divayjindal95
Copy link

@qinhanmin2014 Sure thanks :)

@divayjindal95
Copy link

@qinhanmin2014 , sorry to ask such a silly question, but i am unable to find any good first issue with no active pr, is there any particular way i could find that out ?

@qinhanmin2014
Copy link
Member Author

@divayjindal95 Try searching for help wanted label and easy label, or simply go through the issue tracker :)

@jnothman
Copy link
Member
jnothman commented Jul 15, 2018 via email

@qinhanmin2014
Copy link
Member Author

Resolved in #11489

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jnothman @ggc87 @divayjindal95 @qinhanmin2014
0