-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support FeatureTransforms #50
Conversation
(:predict, :price) => [0.25 1.0; 25.0 4.0; 0.0 1.0] | ||
``` | ||
""" | ||
function FeatureTransforms.apply(ds::KeyedDataset, t::Transform, key=Pattern((:__,)); kwargs...) |
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 remove dims
because it is already being used in the underlying FeatureTransforms
methods so I don't think we should be using for special purposes here. At least not for now.
For FeatureTransforms compat
Also update docstrings of apply and _apply_paths
- Map simplifies things - don't need the custom _apply_paths() anymore - Returning full dataset seems more appropriate after talking to Rory
This reverts commit f85a135.
77cf9f1
to
d99a661
Compare
Codecov Report
@@ Coverage Diff @@
## main #50 +/- ##
==========================================
- Coverage 98.93% 98.60% -0.34%
==========================================
Files 7 9 +2
Lines 283 287 +4
==========================================
+ Hits 280 283 +3
- Misses 3 4 +1
Continue to review full report at Codecov.
|
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, I'm not sure if this'll cover all our use cases, but it seems like a reasonable first pass.
2553617
to
52c6b0c
Compare
branched off https://github.com/invenia/AxisSets.jl/tree/bc/featuretransforms and partially replaces #44
apply!
andapply_append
methods for now since all we need isapply
for KeyedDataset to beis_transformable
key
argument for similar reasons - now it looks more likeImpute.apply
method so I also replaced the original_pattern
methods.