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

[MRG+1] TST apply transformer checks to any estimator supporting 'transform' #10474

Merged
merged 5 commits into from Feb 9, 2018

Conversation

Projects
None yet
3 participants
@jnothman
Member

jnothman commented Jan 15, 2018

I found that we were only applying common tests for transformers to those inheriting TransformerMixin. This seems strange to me. Let's try ducktyping it.

TODO:

  • see if tests pass and/or decide if fit_transform would be a better duck typing check
  • test case

@jnothman jnothman changed the title from TST apply transformer checks to any estimator supporting 'transform' to [WIP] TST apply transformer checks to any estimator supporting 'transform' Jan 15, 2018

jnothman added some commits Jan 15, 2018

@jnothman jnothman changed the title from [WIP] TST apply transformer checks to any estimator supporting 'transform' to [MRG] TST apply transformer checks to any estimator supporting 'transform' Jan 16, 2018

@glemaitre

This comment has been minimized.

Contributor

glemaitre commented Feb 8, 2018

LGTM

@glemaitre glemaitre changed the title from [MRG] TST apply transformer checks to any estimator supporting 'transform' to [MRG+1] TST apply transformer checks to any estimator supporting 'transform' Feb 8, 2018

@glemaitre

This comment has been minimized.

Contributor

glemaitre commented Feb 8, 2018

@TomDLT @lesteve @rth Could you have a look at it

@TomDLT

This comment has been minimized.

Member

TomDLT commented Feb 8, 2018

Don't forget to update the whats_new on estimator check

otherwise LGTM

@jnothman

This comment has been minimized.

Member

jnothman commented Feb 9, 2018

@jnothman jnothman merged commit b90661d into scikit-learn:master Feb 9, 2018

8 checks passed

ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: python2 Your tests passed on CircleCI!
Details
ci/circleci: python3 Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.2%)
Details
codecov/project 96.2% (+<.01%) compared to 4a2b96f
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details
@jnothman

This comment has been minimized.

Member

jnothman commented Feb 9, 2018

Thanks for the reviews

cperales added a commit to cperales/scikit-learn that referenced this pull request Feb 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment