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] Export ClassificationCriterion and RegressionCriterion #10325

Merged
merged 1 commit into from Dec 18, 2017

Conversation

Projects
None yet
5 participants
@camilstaps
Contributor

camilstaps commented Dec 14, 2017

Reference Issues/PRs

Resolves #10251.

What does this implement/fix? Explain your changes.

This allows extending of ClassificationCriterion and RegressionCriterion. For an example use case, see https://stats.stackexchange.com/q/316954/98500.

Any other comments?

I wasn't sure what to do with the documentation. The documentation on Criterion is different in the pyx and the pxd, and the latter is not a docstring. I copied the very succinct docstring for ClassificationCriterion and wrote a similar one for RegressionCriterion. But as Criterion does something weird, which I cannot find in the coding guidelines, I don't know what I should do there. Advise appreciated.

Because this is not a public API, I did not provide usage examples. (A usage example would by any of the subclasses defined in the pyx.) Please let me know if they should be added.

Thanks for reviewing!

@jnothman

This comment has been minimized.

Member

jnothman commented Dec 18, 2017

I'm fine with this as an intermediate step. LGTM.

@jnothman jnothman changed the title from [MRG] Export ClassificationCriterion and RegressionCriterion to [MRG+2] Export ClassificationCriterion and RegressionCriterion Dec 18, 2017

@jnothman jnothman changed the title from [MRG+2] Export ClassificationCriterion and RegressionCriterion to [MRG+1] Export ClassificationCriterion and RegressionCriterion Dec 18, 2017

@glemaitre glemaitre merged commit d523243 into scikit-learn:master Dec 18, 2017

6 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 61e722a...a09bde2
Details
codecov/project 96.17% remains the same compared to 61e722a
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
@glemaitre

This comment has been minimized.

Contributor

glemaitre commented Dec 18, 2017

LGTM. Merging ... thanks @camilstaps

@camilstaps camilstaps deleted the camilstaps:10251-extend-criterion branch Dec 18, 2017

@rajday19

This comment has been minimized.

rajday19 commented May 22, 2018

@camilstaps
Is the extension for the regression criterion part of the main scikit-learn?

I need to create a custom criterion function for the RandomForestRegressor. Can you provide an example of how to pass a custom function using the new patch? Much appreciated.

@camilstaps

This comment has been minimized.

Contributor

camilstaps commented May 23, 2018

@rajday19 This PR only facilitates extensions, it didn't add any extensions itself.

I'm afraid I can't help much with writing the custom criterion, see the discussion at the end of #10251.

@EvgeniDubov

This comment has been minimized.

EvgeniDubov commented Oct 9, 2018

@glemaitre
I'm working on Hellinger Distance Criterion cython implementation of ClassificationCriterion in imblearn #437
For the new criterion implementation I need to inherit the ClassificationCriterion using this line:
from sklearn.tree._criterion cimport ClassificationCriterion
In order for this line to build successfully I need _criterion.pxd file to be located in sklearn\tree
However when I install scikit-learn 0.20 the .pxd is not deployed and I get the following error during cython build:

from sklearn.tree._criterion cimport ClassificationCriterion
^
------------------------------------------------------------
imblearn\tree\criterion.pyx:10:0: 'sklearn\tree\ _criterion.pxd' not found

I went through sklearn setup files and MANIFEST and looks like the .pxd should have got into the whl and be deployed

Can you please direct me on the proper way to inherit the exported ClassificationCriterion?

Thank you

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