Skip to content

Conversation

@stepansergeevitch
Copy link
Collaborator

Added support for decimal, date32 and datetime64 types.
Added unit tests and for decimal added integration tests

Copy link
Contributor

@ptiurin ptiurin left a comment

Choose a reason for hiding this comment

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

We should probably add integration tests for extended date and timestamps?

"1.0387398573",
"some text",
"2019-07-31",
"2020-07-31",
Copy link
Contributor

Choose a reason for hiding this comment

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

New dates extended the range below 1970 so let's reflect this in a test (despite it being a unit test) e.g. 1860-01-31, which was not possible with regular DATE

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, agree

Comment on lines -95 to -96
assert parse_value("2021-12-31 23:59:59", datetime) == datetime(
2021, 12, 31, 23, 59, 59
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the same test be there for date32?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

date32 doesn't support milliseconds

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I see what you mean

Copy link
Contributor

@ptiurin ptiurin left a comment

Choose a reason for hiding this comment

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

lgtm

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

88.2% 88.2% Coverage
0.0% 0.0% Duplication

@stepansergeevitch stepansergeevitch merged commit 9a3e76f into main Mar 10, 2022
@stepansergeevitch stepansergeevitch deleted the FIR-11238-python-sdk-support-new-data-ty branch March 10, 2022 11:33
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.

4 participants