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

[dtypes] Numerical Formatter Fails to Learn Format of New Data Types #2165

Merged
merged 6 commits into from
Aug 7, 2024

Conversation

R-Palazzo
Copy link
Contributor

@R-Palazzo R-Palazzo commented Aug 2, 2024

CU-86b1he00a
CU-86b1gc5jb
Resolve #2156
Resolve #2164
Resolve #2157

#2157 seems to be directly solved by this PR:
Screenshot 2024-08-05 at 16 11 05

@R-Palazzo R-Palazzo self-assigned this Aug 2, 2024
@R-Palazzo R-Palazzo requested a review from a team as a code owner August 2, 2024 13:58
@R-Palazzo R-Palazzo removed the request for review from a team August 2, 2024 13:58
@@ -55,6 +55,8 @@ class SingleTableMetadata:
}

_NUMERICAL_REPRESENTATIONS = frozenset([
'Float32',
'Float64',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't add a test for this because there is already this test to check the valid computer_representation:

@pytest.mark.parametrize(

Copy link
Contributor

@amontanez24 amontanez24 left a comment

Choose a reason for hiding this comment

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

LGTM!



def test_support_new_pandas_dtypes():
"""Test that the synthesizer supports the new pandas dtypes."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Test that the synthesizer supports the new pandas dtypes."""
"""Test that the synthesizer supports the nullable numerical pandas dtypes."""

@R-Palazzo R-Palazzo force-pushed the issue-2156-numericalformatter branch from 05a3ebe to d608cee Compare August 6, 2024 16:17
@R-Palazzo R-Palazzo merged commit 0517e68 into main Aug 7, 2024
39 checks passed
@R-Palazzo R-Palazzo deleted the issue-2156-numericalformatter branch August 7, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants