Skip to content

Support Woodwork's update numeric inference (integers as strings)#2505

Merged
gsheni merged 7 commits intomainfrom
fix_int_cast
Mar 9, 2023
Merged

Support Woodwork's update numeric inference (integers as strings)#2505
gsheni merged 7 commits intomainfrom
fix_int_cast

Conversation

@gsheni
Copy link
Copy Markdown
Contributor

@gsheni gsheni commented Mar 9, 2023

@gsheni gsheni requested a review from thehomebrewnerd March 9, 2023 16:33
@gsheni gsheni self-assigned this Mar 9, 2023
@gsheni gsheni marked this pull request as ready for review March 9, 2023 16:34
df = pd.DataFrame(
{
"id": ["0", "1", "2"],
"id": [0, 1, 2],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think maybe we just want to set the LogicalType for ID on L943 (and remove L946) instead of relying on inference here? Maybe we should just set all the types regardless of the dataframe type. I'm not sure we need to rely on WW inference for anything in this test.

Comment thread featuretools/tests/entityset_tests/test_es.py Outdated
Comment thread featuretools/tests/entityset_tests/test_es.py Outdated
@gsheni gsheni requested a review from thehomebrewnerd March 9, 2023 17:19
@gsheni gsheni enabled auto-merge (squash) March 9, 2023 17:19
Copy link
Copy Markdown
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.

Looks good, assuming CI passes

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2023

Codecov Report

Merging #2505 (6d30287) into main (eab4527) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2505   +/-   ##
=======================================
  Coverage   99.44%   99.44%           
=======================================
  Files         367      367           
  Lines       21882    21882           
=======================================
  Hits        21760    21760           
  Misses        122      122           
Impacted Files Coverage Δ
featuretools/tests/entityset_tests/test_es.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@gsheni gsheni disabled auto-merge March 9, 2023 18:38
@gsheni gsheni merged commit b625de7 into main Mar 9, 2023
@gsheni gsheni deleted the fix_int_cast branch March 9, 2023 18:51
@ozzieD ozzieD mentioned this pull request Mar 28, 2023
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.

Support Woodwork's update numeric inference (integers as strings)

2 participants