make null lowercase a missing value #16534
Conversation
|
|
Thanks,
1. will add it to parsers.pyx - _NA_VALUES line 279
2. na_values.py - added to the test, method test_default_na_values() , should be in the pull request.BTW was not able to run pytest, maybe because the test class name doesn't start with the word test;wrote my own simple test with the data that triggered this issue, it passed.
On Monday, May 29, 2017 4:15 PM, gfyoung <[email protected]> wrote:
- You need to add it to parsers.pyx
- There's a test (also in na_values.py) that checks whether each NaN value you specified gets interpreted as such. You should add null to that test.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@OlegShteynbuk : Yeah, you need to run |
|
run pytest twice, second time with the class name, some tests skipped, all others passed, should be ok, right?
pytest pandas/tests/io/parser/test_parsers.
725 passed, 13 skipped in 76.04 seconds
pytest pandas/tests/io/parser/test_parsers.py::TestPythonParser
235 passed, 5 skipped in 30.06 seconds
On Monday, May 29, 2017 4:29 PM, gfyoung <[email protected]> wrote:
@OlegShteynbuk : Yeah, you need to run pytest test_parsers.py instead (and yeah, that was the test I was referring to. Good catch 😄 )—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@OlegShteynbuk : Nope, not an issue. |
|
added it to parsers.pyx thanks to gfyoung |
Codecov Report
@@ Coverage Diff @@
## master #16534 +/- ##
=======================================
Coverage 90.79% 90.79%
=======================================
Files 161 161
Lines 51063 51063
=======================================
Hits 46365 46365
Misses 4698 4698
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16534 +/- ##
=======================================
Coverage 90.79% 90.79%
=======================================
Files 161 161
Lines 51063 51063
=======================================
Hits 46365 46365
Misses 4698 4698
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16534 +/- ##
==========================================
+ Coverage 90.74% 90.75% +<.01%
==========================================
Files 161 161
Lines 51074 51073 -1
==========================================
+ Hits 46349 46350 +1
+ Misses 4725 4723 -2
Continue to review full report at Codecov.
|
|
@OlegShteynbuk : See the checklist in the PR you submitted? Just run through those and make sure you can check all of them off. Then it should be good to merge! |
|
@gfyoung looks ok, not sure if the last test change to parsers.pyx is included, or need a new pull request. |
|
I'm not sure you fully understood what I was asking you to do. The code changes are fine, but there are several other things you need to do (i.e whatsnew) before this will be merged. That's why we have the checkboxes in the PR you submitted. |
|
where are the checkboxes located, can see checks for tests they all fine, i can write whatsnew but where is it? |
|
If you look at the web version of GitHub, the very first text box under the PR title has the checkboxes. Also, the whatsnew can be found under |
|
there are checkboxes on the top, first comment, there is whatsnew checkbox don't see any links, need to add whatsnew somewhere |
Correct. Make sure you've satisfied everything listed on those checkboxes.
Yes, I gave you the file-path that you should follow in your local repository: |
|
probably need a new .txt file under pandas/doc/source/whatsnew |
|
No, you shouldn't. You should add it to the 0.21 version. |
|
ok, will add 0.21 |
|
added to whatsnew 0.21, commited to my branch |
|
all other checkboxes looks fine, will be away for about an hour, do i need to do anything else before leaving? |
|
|
probably need to modify na_values in io.rst file too as it goes into docs |
jreback
added
IO CSV
doc/source/whatsnew/v0.21.0.txt
Outdated
| @@ -38,6 +38,7 @@ Other Enhancements | |||
| - :func:`read_feather` has gained the ``nthreads`` parameter for multi-threaded operations (:issue:`16359`) | |||
| - :func:`DataFrame.clip()` and :func: `Series.cip()` have gained an inplace argument. (:issue: `15388`) | |||
| - :func:`crosstab` has gained a ``margins_name`` parameter to define the name of the row / column that will contain the totals when margins=True. (:issue:`15972`) | |||
| - The ``na_values`` argument for :func:`read_csv` function now has 'null' (in lower case) as a default value that is interpreted as NaN: (:issue:`16471`) | |||
There was a problem hiding this comment.
:func:`read_csv has gained 'null' as an additional default missing value.
na_values is not involved here, rather the default has changed.
There was a problem hiding this comment.
changed; yes - na_values is just a way to add additional strings to those default values for one function call, thanks for the correction.
OlegShteynbuk
force-pushed
the
null_lower_case
branch
from
04b7dbb to
da74438
Compare
|
thanks @OlegShteynbuk |
|
@gfyoung do we have an issue about consolidating the duplicate definitions of NA values (in parsers & IO), maybe just remove from parsers and leave in IO is best. if not, can you create one? |
|
consolidated the duplicate definitions of NA values (in parsers & IO), removed from parsers and left in IO, assert removed too; probably need to create a different issue. |
|
consolidated NA values in parsers.pyx |
xref #16079
closes #16471