-
Notifications
You must be signed in to change notification settings - Fork 178
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
Added support for multioutputs to ResponseProcesser, with tests. #150
Conversation
This should allow us to convert CSVLoader to use CSVDataSource as its backend, increasing consistency and simplicity.
…rocessor` if response fields were missing from the input rows.
… use `=` to match usage in `MultiLabel`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bunch of small things and a couple of correctness issues.
Data/src/main/java/org/tribuo/data/columnar/ResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/ResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/ResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/BinaryResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/BinaryResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/QuartileResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/QuartileResponseProcessor.java
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/QuartileResponseProcessor.java
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/QuartileResponseProcessor.java
Outdated
Show resolved
Hide resolved
...test/java/org/tribuo/data/columnar/processors/response/MultioutputResponseProcessorTest.java
Outdated
Show resolved
Hide resolved
I believe I've addressed all the changes requested with this commit, let me know if I missed something. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the quartile one needs another look to make sure the behaviour hasn't changed, but otherwise it's pretty close.
Data/src/main/java/org/tribuo/data/columnar/ResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/BinaryResponseProcessor.java
Outdated
Show resolved
Hide resolved
Data/src/main/java/org/tribuo/data/columnar/processors/response/QuartileResponseProcessor.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Description
Changes
ResponseProcessor
s to process response inputs for multiple fields, supporting things like multilabel or multiclass classification.Motivation
This is a new feature that extends the generality of the
org.tribuo.data.columnar
system, but the direct motivation is to allowCSVLoader
to become a special case ofCSVDataSource
, by supporting all of the features ofCSVLoader
withinColumnarDataSource
.