Repository navigation
Give PandasCursor's temporal columns one dtype per Athena type #1273
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0967010
Give PandasCursor's temporal columns one dtype per Athena type
laughingman7743 b752a34
Describe PandasCursor's temporal dtypes and fetched types
laughingman7743 b765f4d
Convert early local times, group zones once, and share the label disp…
laughingman7743 d9a4ba0
Test early local times, conversion errors, engine-independent time zo…
laughingman7743 244ae20
Describe the temporal conversion precedence, errors, and iterator rows
laughingman7743 7afc37d
Ignore pandas' converter-and-dtype warning in the engine test
laughingman7743 acf5baa
Keep NULL in every engine, take positional and mapping options, and l…
laughingman7743 3da05c4
Resolve dtype and converters keys of temporal columns as pandas does
laughingman7743 bab3840
Test that the PyArrow engine takes no dtype positions of temporal col…
laughingman7743 36b23f0
Leave temporal columns to pandas with parse_dates, dtype, or converters
laughingman7743 b59362f
Treat a None parse_dates, dtype, or converters as not given
laughingman7743 d39163c
Check that converters of None keep the PyArrow engine
laughingman7743 3b62b8d
Expect datetime64 for the UNLOAD date column of the complex pyarrow test
laughingman7743 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Self-review of the rule change,
9db79526..f7a5e96d(one added commit, same base).Round 1 (behavior and test quality): CLEAN.
_get_csv_temporal_convertersreturns no converters whenparse_dates,dtype, orconvertersis given, and the PyArrow engine condition follows the same rule; the removed code was only the key lookup. The new tests check the rule through the DataFrame and fetched values for each engine, and fail under each mutation of the rule (offline).Round 2 (claims): FINDINGS, corrected in the PR body: the precedence bullet states the rule and the base behavior that remains with those options, including the
OperationalErrorwithout the column name for an unknown zone in the per-value converter; the tests are twelve and 261 offline tests pass at f7a5e96. This docs paragraph matches the code and keeps the earlier sentences thatdtypereplaces the whole mapping andconvertersreplace the converter mapping.just lintandjust docs lintpassed.