Add an optional pipeline attribute to the Learner object #474
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 a custom densification stage for when we are using feature hashing and doing scaling. - Move sampler to the right place in the pipeline steps.
# Conflicts: # skll/learner.py
|
Okay, this PR is now ready for review @jbiggsets @mulhod @bndgyawali @Lguyogiro. |
|
Some of the coverage decrease is expected but the rest doesn't make any sense to me. It says that these 2 lines are newly uncovered but we have never had tests for that. |
- No need try and convert already dense arrays to dense.
|
Okay, that's more like it! This drop is indeed expected - I added a whole bunch of new tests to cover things we had never covered before (e.g., Now, this is really ready for review! @jbiggsets @mulhod @bndgyawali! |
mulhod
requested changes
Mar 11, 2019
- Simplify code to use `Reader.for_path(...)` everywhere. - Update note to be much more detailed abiout the densifier vs. turning `sparse` off. - Add code that would be required if there was no pipeline attribute for contrast.
mulhod
approved these changes
Mar 12, 2019
jbiggsets
approved these changes
Mar 13, 2019
This was referenced Mar 13, 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.
This PR addresses 1 major and 2 minor issues.
Issue #451
pipelineattribute to the Learner object if it was enabled either via a keyword argument to theLearner()call or as a config option in the Output section of a config file. If enabled, this attribute contains a scikit-learn Pipeline object containing all of the components that were used (vectorizer, selector, scaler, sampler, and the final estimator) that have already been fit as part of the SKLL training process. In some cases, a custom pipeline stage to force sparse feature vectors to dense is needed (see documentation).pipelineconfig option.Issue #472
Learner.predict()where sampling was being applied before the scaling.Issue #473
MultinomialNBlearner since it cannot handle negative feature values.