Skip to content

Make -NaN and -nan default NA values #6038

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 1 commit into from
Jan 23, 2014
Merged

Make -NaN and -nan default NA values #6038

merged 1 commit into from
Jan 23, 2014

Conversation

Bklyn
Copy link
Contributor

@Bklyn Bklyn commented Jan 22, 2014

closes #5952

@dsm054
Copy link
Contributor

dsm054 commented Jan 22, 2014

Probably test_default_na_values should be brought into alignment?

@jreback
Copy link
Contributor

jreback commented Jan 22, 2014

@Bklyn yep on test_default_na_values...in theory that should fail with your change (until its fixed)....does it?

@Bklyn
Copy link
Contributor Author

Bklyn commented Jan 22, 2014

Sorry for the noob-ishness, but what is the suggested way to run the tests against not-yet-installed version of pandas (e.g. from my working copy)? I'm getting import errors if I run the test script by hand unless I "setup.py install" first.

At any rate, the tests do not fail if I don't change them, presumably because they're not trying the new -nan values. Will fix the code and update the PR with the tests for completeness.

@jreback
Copy link
Contributor

jreback commented Jan 22, 2014

@Bklyn make this part of the same test (which SHOULD fail if the _NA_VALUES are not updated)

e.g.

assert_array_equal(_NA_VALUES,pandas.io.parsers._NA_VALUES)

This way the test have to be updated as well

@jreback
Copy link
Contributor

jreback commented Jan 22, 2014

@Bklyn

python setup.py build

then

nosetests pandas/io/tests/test_parser.py

@Bklyn
Copy link
Contributor Author

Bklyn commented Jan 23, 2014

Is there anything left for me to do on this PR?

@jreback
Copy link
Contributor

jreback commented Jan 23, 2014

pls add a release notes entry (put in 0.13.1 API),

rebase and squash to 1 commit: https://github.com/pydata/pandas/wiki/Using-Git

@@ -104,6 +104,10 @@ Enhancements
result
result.loc[:,:,'ItemA']

- Add ``-NaN`` and ``-nan`` to the default set of NA values (:issue:`5952`).
Copy link
Contributor

Choose a reason for hiding this comment

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

update this too (actually add to the text): http://pandas.pydata.org/pandas-docs/dev/io.html#na-values

@Bklyn
Copy link
Contributor Author

Bklyn commented Jan 23, 2014

OK, I think thats got it.

@@ -18,6 +18,9 @@ There are several new or updated docs sections including:
API changes
~~~~~~~~~~~

- Add ``-NaN`` and ``-nan`` to the default set of NA values (:issue:`5952`).
Copy link
Contributor

Choose a reason for hiding this comment

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

just add this line also to doc/source/release.rst under API changes as well. we use the whatsnew to 'announce' important changes, and the release.rst as a log of them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be there now.

@jreback
Copy link
Contributor

jreback commented Jan 23, 2014

@y-p @jtratner ok by you?

@ghost
Copy link

ghost commented Jan 23, 2014

fine by me.

jreback added a commit that referenced this pull request Jan 23, 2014
Make -NaN and -nan default NA values
@jreback jreback merged commit 12c8793 into pandas-dev:master Jan 23, 2014
@jreback
Copy link
Contributor

jreback commented Jan 23, 2014

thanks @Bklyn

nice little PR, and you were very responsive!!

@Bklyn
Copy link
Contributor Author

Bklyn commented Jan 23, 2014

Thanks for the hand-holding and the feedback. You guys (and perhaps gals?) do a great job with pandas and your attention to code and documentation quality is impressive.

@jreback
Copy link
Contributor

jreback commented Jan 23, 2014

welcome! feel free to tackle any open issues!

@ghost
Copy link

ghost commented Jan 24, 2014

Yeah, we need a gal commiter.

Well, a ninja that happens to be a gal.

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.

Negative NaN (-nan) in CSV input treated as object
3 participants