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

Add RollingTrend primitives and refactor some util methods #2170

Merged
merged 25 commits into from
Jul 19, 2022

Conversation

ozzieD
Copy link
Contributor

@ozzieD ozzieD commented Jul 5, 2022

Resolves: #1806

also fix code coverage issues

@ozzieD ozzieD changed the title Draft: add RollingTrend primitives and refactor some util methods add RollingTrend primitives and refactor some util methods Jul 11, 2022
@gsheni gsheni changed the title add RollingTrend primitives and refactor some util methods Add RollingTrend primitives and refactor some util methods Jul 11, 2022
Copy link
Contributor

@thehomebrewnerd thehomebrewnerd left a comment

Choose a reason for hiding this comment

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

Here are a couple small comments after a quick scan - need a little more time to digest what is happening in the new primitive.

featuretools/primitives/api.py Outdated Show resolved Hide resolved
featuretools/tests/primitive_tests/test_primitive_utils.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 12, 2022

Codecov Report

Merging #2170 (cccd725) into main (cd449e8) will increase coverage by 0.03%.
The diff coverage is 99.47%.

@@            Coverage Diff             @@
##             main    #2170      +/-   ##
==========================================
+ Coverage   99.26%   99.29%   +0.03%     
==========================================
  Files         143      145       +2     
  Lines       17506    17556      +50     
==========================================
+ Hits        17377    17432      +55     
+ Misses        129      124       -5     
Impacted Files Coverage Δ
featuretools/primitives/utils.py 99.51% <ø> (-0.10%) ⬇️
...ools/tests/primitive_tests/test_primitive_utils.py 100.00% <ø> (ø)
featuretools/utils/time_utils.py 95.00% <94.59%> (-0.66%) ⬇️
featuretools/primitives/rolling_primitive_utils.py 100.00% <100.00%> (ø)
...ools/primitives/standard/aggregation_primitives.py 99.68% <100.00%> (+2.52%) ⬆️
...primitives/standard/rolling_transform_primitive.py 100.00% <100.00%> (ø)
...ls/tests/primitive_tests/test_rolling_primitive.py 100.00% <100.00%> (ø)
...ts/primitive_tests/test_rolling_primitive_utils.py 100.00% <100.00%> (ø)
featuretools/tests/utils_tests/test_time_utils.py 96.61% <100.00%> (+1.05%) ⬆️
featuretools/utils/api.py 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd449e8...cccd725. Read the comment docs.

tamargrey
tamargrey previously approved these changes Jul 14, 2022
Copy link
Contributor

@tamargrey tamargrey left a comment

Choose a reason for hiding this comment

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

LGTM!

sbadithe
sbadithe previously approved these changes Jul 16, 2022
@sbadithe
Copy link
Contributor

Woodwork tests should fix hopefully with #2182!

gsheni
gsheni previously approved these changes Jul 18, 2022
@ozzieD ozzieD dismissed stale reviews from gsheni and sbadithe via b344026 July 18, 2022 15:03
@ozzieD ozzieD merged commit 8057ce5 into main Jul 19, 2022
@ozzieD ozzieD deleted the 1806-add-rollingtrend-primitive branch July 19, 2022 01:08
This was referenced Jul 19, 2022
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.

Add a RollingTrend transform primitive
6 participants