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

Correct treatment of the unseen labels in the training set #290

Merged
merged 5 commits into from
Feb 19, 2016

Conversation

aloukina
Copy link
Collaborator

No description provided.


def test_all_new_labels_in_test():
"""
Test classification with all labes in test set unseen
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"labes" -> "labels"

@desilinguist
Copy link
Member

Looks pretty good. Just a trivial typo.

model_params,
grid_score) = res

# check that the new metric is included into the results
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean label instead of metric?

@desilinguist
Copy link
Member

@aloukina you need to merge in master as well.

@desilinguist
Copy link
Member

It this ready, @aloukina?

@aloukina
Copy link
Collaborator Author

See my comment under #271 - we need to make sure that the labels in the confusion matrix are assigned correctly when the new labels fall between the old labels. Currently my branch generates confusion matrix with new labels at the end of the matrix. I have not looked at that part of the code yet to see whether we need to fix this here or there.

@desilinguist
Copy link
Member

I think that should be fixed in #271 after merging this one. Your PR by itself is ready right?

@aloukina
Copy link
Collaborator Author

yes

@desilinguist
Copy link
Member

@aoifecahill since you filed this issue, is there some kind of other data you can test on to make sure this is working as expected?

@aoifecahill
Copy link
Collaborator

Yep, will do.

@aoifecahill
Copy link
Collaborator

Works on my dataset 👍

desilinguist added a commit that referenced this pull request Feb 19, 2016
…low-unseen-labels-in-the-test-set

Correct treatment of the unseen labels in the training set
@desilinguist desilinguist merged commit b5ba26a into master Feb 19, 2016
@desilinguist desilinguist deleted the feature/279-allow-unseen-labels-in-the-test-set branch February 19, 2016 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants