Skip to content

[MRG+1] Export ClassificationCriterion and RegressionCriterion#10325

Merged
glemaitre merged 1 commit intoscikit-learn:masterfrom
camilstaps:10251-extend-criterion
Dec 18, 2017
Merged

[MRG+1] Export ClassificationCriterion and RegressionCriterion#10325
glemaitre merged 1 commit intoscikit-learn:masterfrom
camilstaps:10251-extend-criterion

Conversation

@camilstaps
Copy link
Copy Markdown
Contributor

@camilstaps 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
Copy link
Copy Markdown
Member

jnothman commented Dec 18, 2017

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

@jnothman jnothman changed the title [MRG] Export ClassificationCriterion and RegressionCriterion [MRG+2] Export ClassificationCriterion and RegressionCriterion Dec 18, 2017
@jnothman jnothman changed the title [MRG+2] Export ClassificationCriterion and RegressionCriterion [MRG+1] Export ClassificationCriterion and RegressionCriterion Dec 18, 2017
@glemaitre glemaitre merged commit d523243 into scikit-learn:master Dec 18, 2017
@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Dec 18, 2017

LGTM. Merging ... thanks @camilstaps

@camilstaps camilstaps deleted the 10251-extend-criterion branch December 18, 2017 17:31
@rajday19
Copy link
Copy Markdown

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
Copy link
Copy Markdown
Contributor Author

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.

This was referenced Jul 10, 2018
Closed
Closed
@EvgeniDubov
Copy link
Copy Markdown

EvgeniDubov commented Oct 9, 2018 •
edited
Loading

@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

@EvgeniDubov EvgeniDubov mentioned this pull request Oct 9, 2018
Open
@glemaitre glemaitre mentioned this pull request Oct 12, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jnothman jnothman jnothman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Extending Criterion

5 participants

@camilstaps @jnothman @glemaitre @rajday19 @EvgeniDubov