Skip to content
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

Enhanced ctree_feature_space() that accepts a features arg rather than requiring a model with just 1 or 2 features. #253

Merged
merged 13 commits into from
Jan 22, 2023

Conversation

parrt
Copy link
Owner

@parrt parrt commented Jan 21, 2023

Signed-off-by: Terence Parr parrt@antlr.org

Signed-off-by: Terence Parr <parrt@antlr.org>
@parrt parrt added the enhancement New feature or request label Jan 21, 2023
@parrt parrt added this to the 2.1.1 milestone Jan 21, 2023
…arg.

Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
@parrt parrt requested a review from mepland January 21, 2023 22:39
@parrt
Copy link
Owner Author

parrt commented Jan 21, 2023

ok, ready for review! I updated sklearn notebook.

@mepland
Copy link
Collaborator

mepland commented Jan 21, 2023

I think we should turn the y-axis on for univar when gtype='barstacked' otherwise you can't tell anything from the magnitude.

image

@parrt
Copy link
Owner Author

parrt commented Jan 22, 2023

I guess it’s just a count, right?

@tlapusan
Copy link
Collaborator

Is there a need to reinitialize the decision tree/dtreeviz in case we want to use one and after that two features ?
I think what @mepland initially suggested is to initialize a decision tree/dtreeviz model and after that to choose if you want to make a viz with one or two features. Like in the bellow screenshot :

Screenshot 2023-01-22 at 15 47 37

@mepland
Copy link
Collaborator

mepland commented Jan 22, 2023

Is there a need to reinitialize the decision tree/dtreeviz in case we want to use one and after that two features ?

I think that is the current behavior, the screenshot looks fine to me?

@tlapusan
Copy link
Collaborator

I took the code from this PR and there is reinitialization for both classification and regression... @mepland can you check this also please ?

Signed-off-by: Terence Parr <parrt@antlr.org>
Signed-off-by: Terence Parr <parrt@antlr.org>
@parrt
Copy link
Owner Author

parrt commented Jan 22, 2023

Good catch @tlapusan. Sorry about that. cleaned up and resubmitted the notebook so that it creates a single decision tree and model, then reuses that for visualization

@mepland
Copy link
Collaborator

mepland commented Jan 22, 2023

I see what you mean now @tlapusan.

@parrt looks good to me now, but I would still like the option of having a y-axis for gtype='barstacked'.

dtreeviz/trees.py Outdated Show resolved Hide resolved
dtreeviz/trees.py Outdated Show resolved Hide resolved
Signed-off-by: Terence Parr <parrt@antlr.org>
@parrt
Copy link
Owner Author

parrt commented Jan 22, 2023

@parrt looks good to me now, but I would still like the option of having a y-axis for gtype='barstacked'.

Fixed

Screenshot 2023-01-22 at 11 15 59 AM

…n the number of model features.

Signed-off-by: Terence Parr <parrt@antlr.org>
@parrt parrt requested a review from mepland January 22, 2023 19:24
@parrt
Copy link
Owner Author

parrt commented Jan 22, 2023

OK, can you guys do one last quick check? If it looks good I can push it for a quick release so I can use it within colab.

dtreeviz/trees.py Outdated Show resolved Hide resolved
…n the number of model features.

Signed-off-by: Terence Parr <parrt@antlr.org>
dtreeviz/trees.py Outdated Show resolved Hide resolved
…n the number of model features.

Signed-off-by: Terence Parr <parrt@antlr.org>
…n the number of model features.

Signed-off-by: Terence Parr <parrt@antlr.org>
@parrt
Copy link
Owner Author

parrt commented Jan 22, 2023

Cool. I will push out a new version today.

@parrt parrt merged commit 8608ba3 into dev Jan 22, 2023
@parrt parrt changed the title Got basics of new ctree_feature_space() that accepts a features arg. Enhanced ctree_feature_space() that accepts a features arg rather than requiring a model with just 1 or 2 features. Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants