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

Support NA values with boolean column (fix #100, fix #102, fix #103) #104

Merged
merged 10 commits into from
Dec 6, 2019

Conversation

laughingman7743
Copy link
Owner

No description provided.

@laughingman7743 laughingman7743 force-pushed the support_na_values_with_boolean_column branch 5 times, most recently from fc3ab3e to 9d680a2 Compare November 23, 2019 14:43
@codecov
Copy link

codecov bot commented Nov 24, 2019

Codecov Report

Merging #104 into master will increase coverage by 0.39%.
The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
+ Coverage   91.99%   92.39%   +0.39%     
==========================================
  Files          14       14              
  Lines        1062     1104      +42     
==========================================
+ Hits          977     1020      +43     
+ Misses         85       84       -1
Impacted Files Coverage Δ
pyathena/result_set.py 92.59% <ø> (-0.22%) ⬇️
pyathena/connection.py 88.6% <100%> (+1.1%) ⬆️
pyathena/util.py 100% <100%> (+11.53%) ⬆️
pyathena/converter.py 86.95% <88.63%> (+6.62%) ⬆️
pyathena/formatter.py 94.93% <88.88%> (-2.04%) ⬇️

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 3eef7d2...03805e5. Read the comment docs.

@laughingman7743 laughingman7743 force-pushed the support_na_values_with_boolean_column branch from 12b5735 to d13c8d3 Compare November 24, 2019 12:35
@laughingman7743 laughingman7743 force-pushed the support_na_values_with_boolean_column branch from d13c8d3 to 16c9e12 Compare November 24, 2019 12:55
self.assertEqual(rows, [
(True, False),
(False, None),
(None, None),

Choose a reason for hiding this comment

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

👍 Nice tests ! I understand that since you dont provide a dtype for the boolean column when calling read_csv. Pandas cast it either to boolean if there are no missing values or object if there are some ?

That would explain why it's a None and not a na.

Thanks a lot for the prompt PR :)

Copy link
Owner Author

Choose a reason for hiding this comment

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

I wrote how to customize dtypes and converters in README.

Copy link
Owner Author

Choose a reason for hiding this comment

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

class DefaultPandasTypeConverter(Converter):

By default this class is used.

@laughingman7743 laughingman7743 force-pushed the support_na_values_with_boolean_column branch from fcb9bae to fdaadde Compare December 1, 2019 06:35
@laughingman7743 laughingman7743 force-pushed the support_na_values_with_boolean_column branch from fdaadde to 03805e5 Compare December 1, 2019 06:57
@laughingman7743 laughingman7743 merged commit d1bd8fe into master Dec 6, 2019
@laughingman7743 laughingman7743 deleted the support_na_values_with_boolean_column branch December 6, 2019 13:13
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.

None yet

2 participants