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.

Sign up for GitHub

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

Jump to bottom

BUG: Don't parse NaN as 'nan' in Data IO #23162

Merged
merged 1 commit into from
Oct 18, 2018
Merged

BUG: Don't parse NaN as 'nan' in Data IO #23162

merged 1 commit into from
Oct 18, 2018

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Oct 15, 2018 •
edited
Loading

Re-implementation of #20429, with a couple of changes:

Closes #20377.

@gfyoung gfyoung added IO Data IO issues that don't fit into a more specific label Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Dtype Conversions Unexpected or buggy dtype conversions labels Oct 15, 2018
@gfyoung gfyoung added this to the 0.24.0 milestone Oct 15, 2018
@pep8speaks
Copy link

Hello @gfyoung! Thanks for submitting the PR.

@codecov
Copy link

codecov bot commented Oct 15, 2018 •
edited
Loading

Codecov Report

Merging #23162 into master will not change coverage.
The diff coverage is 75%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #23162   +/-   ##
=======================================
  Coverage   92.19%   92.19%           
=======================================
  Files         169      169           
  Lines       50954    50954           
=======================================
  Hits        46975    46975           
  Misses       3979     3979
Flag Coverage Δ
#multiple 90.61% <75%> (ø) ⬆️
#single 42.27% <50%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/io/parsers.py 95.6% <100%> (ø) ⬆️
pandas/core/dtypes/cast.py 89.22% <66.66%> (ø) ⬆️

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 1546c35...ddd1be3. Read the comment docs.

jreback
jreback requested changes Oct 15, 2018
View reviewed changes
doc/source/whatsnew/v0.24.0.txt Outdated Show resolved Hide resolved
doc/source/whatsnew/v0.24.0.txt Outdated Show resolved Hide resolved
pandas/io/parsers.py Outdated Show resolved Hide resolved
@gfyoung gfyoung force-pushed the read-csv-nan-string branch from a55e8d0 to 5285f5b Compare October 16, 2018 19:11
@WillAyd
Copy link
Member

WillAyd commented Oct 16, 2018

Haven't reviewed in detail yet but this may also close #21131

@gfyoung
Copy link
Member Author

gfyoung commented Oct 16, 2018

Looks like Azure is failing on master but could be addressed by #23182.

@gfyoung gfyoung force-pushed the read-csv-nan-string branch from 5285f5b to dea9b7c Compare October 16, 2018 20:49
jreback
jreback requested changes Oct 17, 2018
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

much nicer this version. just a couple of doc-strings. IN a followup, you can see if you can change the default in astype_nansafe to skipna=True but that might affect other parts of the codebase.

pandas/_libs/lib.pyx Show resolved Hide resolved
pandas/_libs/lib.pyx Show resolved Hide resolved
@gfyoung gfyoung force-pushed the read-csv-nan-string branch from dea9b7c to 524dbb3 Compare October 17, 2018 22:37
@gfyoung gfyoung force-pushed the read-csv-nan-string branch from 524dbb3 to ddd1be3 Compare October 18, 2018 02:55
@gfyoung
Copy link
Member Author

gfyoung commented Oct 18, 2018

@jreback : Addressed all comments, and all is green again! PTAL.

@jreback jreback merged commit 0c51d4d into pandas-dev:master Oct 18, 2018
@jreback
Copy link
Contributor

jreback commented Oct 18, 2018

thanks @gfyoung very nice, keep em coming!

@gfyoung gfyoung deleted the read-csv-nan-string branch October 18, 2018 16:10
tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
@gfyoung @tm9k1
BUG: Don't parse NaN as 'nan' in Data IO (pandas-dev#23162)
06139bc 
Closes pandas-devgh-20377.
jbrockmendel
jbrockmendel reviewed Dec 6, 2022
View reviewed changes
pandas/io/parsers.py
@@ -1685,7 +1685,8 @@ def _cast_types(self, values, cast_type, column):

else:
try:
values = astype_nansafe(values, cast_type, copy=True)
values = astype_nansafe(values, cast_type,
Copy link
Member

Choose a reason for hiding this comment

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

@gfyoung this is a longshot but any idea why you passed skipna=True here but not 9 lines up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrockmendel jbrockmendel jbrockmendel left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Dtype Conversions Unexpected or buggy dtype conversions IO Data IO issues that don't fit into a more specific label Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

read_excel with dtype=str converts empty cells to the string 'nan'
5 participants
@gfyoung @pep8speaks @WillAyd @jreback @jbrockmendel