Properly handle unlabeled data in multiple places in SKLL #453
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add `--no_labels` option to `skll_convert` - Make `--no_labels` and `--label_col` mutually exclusive and add test for this. - Remove `.ndj` from conversion test since it's identical to `.jsonlines` and just adds to the test time.
We do not need to add/remove `label_col` from the fieldnames if it's None to begin with.
- Remove unnecessary test and associated files.
# Conflicts: # skll/data/featureset.py
|
@Lguyogiro @mulhod @jbiggsets any chance of reviewing this soon? I have another branch ready :) |
|
I will take a look today. |
mulhod
reviewed
Feb 11, 2019
There was a problem hiding this comment.
Looks good.
I had a question about the case where you have unlabelled data being converted. You would now use the --no_labels flag, but what happens if you don't use it and there is no label column (y if unspecified)? In the csv to arff case, this still works, but it's probably not doing what we want. I would expect that if you don't pass in --no_labels and there is no label column, it would fail.
|
This is because we allow the CSV reader to ignore non-existent columns and set the label to |
mulhod
approved these changes
Feb 11, 2019
This was referenced Feb 11, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow
skll_convertto handle unlabaled input data (skll_convertdoes not handle conversion of unlabeled data correctly in all cases #452)--no_labelsoption toskll_convert--no_labelsand--label_colmutually exclusive and add test for this.Remove the
.ndjformat from the various conversion tests since it's identical to the.jsonlinesformat and just adds unnecessarily to the test run time.Fix
FeatureSet.has_labelsto recognize list ofNoneobjects which is what happens when you read in an unlabeled data set and passlabel_col=None(Reader/Writers not totally compatible for unlabelled feature sets #426).Fix bug in
ARFFWriterthat adds/removeslabel_colfrom the field names even if it's None to begin with.Update
test_convert_featureset()intest_featuresets.pyto also test for unlabeled data.