consolidated the duplicate definitions of NA values (in parsers & IO) #16589

OlegShteynbuk · 2017-06-03T03:53:28Z

codecov · 2017-06-03T04:26:57Z

Codecov Report

Merging #16589 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #16589      +/-   ##
==========================================
- Coverage   90.95%   90.92%   -0.03%     
==========================================
  Files         161      161              
  Lines       49276    49276              
==========================================
- Hits        44817    44805      -12     
- Misses       4459     4471      +12

Flag	Coverage Δ
#multiple	`88.68% <ø> (-0.03%)`	⬇️
#single	`40.18% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/plotting/_converter.py	`63.23% <0%> (-1.82%)`	⬇️

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 d298414...10cba2c. Read the comment docs.

jreback · 2017-06-03T12:55:28Z

pandas/tests/io/parser/na_values.py

-                          '#N/A', 'N/A', 'n/a', 'NA', '#NA', 'NULL', 'null',
-                          'NaN', 'nan', '-NaN', '-nan', '#N/A N/A', ''])
-        assert _NA_VALUES == parsers._NA_VALUES
+        _NA_VALUES = parsers._NA_VALUES


this needs to stay it asserts that nothing has changed

reverted the change.

jreback · 2017-06-03T12:56:10Z

pandas/_libs/parsers.pyx

@@ -273,12 +275,16 @@ cdef extern from "parser/io.h":

 DEFAULT_CHUNKSIZE = 256 * 1024

+
+def c_type_conv(st):
+    cdef bytes py_bytes = st.encode()


i think we have a routine already like this
look around

thanks, i think have found the function
_NA_VALUES = _ensure_encoded(parsers._NA_VALUES)
then even list comprehension is not needed; need to double check as haven't used Cython before

another function that can be used is asbytes

jreback · 2017-06-05T10:07:56Z

@gfyoung look ok to you?

gfyoung · 2017-06-05T14:38:10Z

Awesome that this works! My only question is whether we should be importing from io.common or from parsers (does it really matter, or does the former not work?)

OlegShteynbuk · 2017-06-05T16:21:26Z

i think it should not matter from the execution point of view as parsers import _NA_VALUES from common, "Explicit is better than implicit." common makes it explicit, on the other hand not sure what is the role of parsers? is it a gatekeeper?

jreback · 2017-06-05T16:24:01Z

importing from io.common is best

OlegShteynbuk · 2017-06-05T16:32:54Z

will change to io.common

gfyoung · 2017-06-05T18:29:01Z

@OlegShteynbuk : nitpicking here: I think we generally use import pandas.io.common as com. However, LGTM otherwise.

OlegShteynbuk · 2017-06-05T18:55:31Z

thanks, changed, was not aware of this

OlegShteynbuk · 2017-06-05T19:28:26Z

got the message - 'No test commands were found", local tests were ok.
not sure what happened, is there a way to repeat the tests without pushing changes?

jreback · 2017-06-06T22:18:40Z

ok restarted circle. ping on green.

OlegShteynbuk · 2017-06-07T23:11:20Z

repetition of the same default NA values in io.rst - removed;

in io.rst just after the defaults are listed there is a sentence

Although a 0-length string
'' is not included in the default NaN values list, it is still treated
as a missing value.

but '' is listed as a default and it is also in common.py is part of the defaults

should i remove this sentence?

jreback · 2017-06-08T12:03:49Z

doc/source/io.rst

@@ -1020,8 +1019,11 @@ the corresponding equivalent values will also imply a missing value (in this cas
 ``[5.0,5]`` are recognized as ``NaN``.

 To completely override the default values that are recognized as missing, specify ``keep_default_na=False``.
-The default ``NaN`` recognized values are ``['-1.#IND', '1.#QNAN', '1.#IND', '-1.#QNAN', '#N/A','N/A', 'NA',


can you build the docs and show a rendering of this page. I think this might generate a build warning (and may not render correctly)

@jorisvandenbossche

I have build the docs locally before commit, there were warnings, some of them might be related to python 3, i have 2.7.13 on linux ; file doc/source/style.ipynb also was a problem, but the generated html looks ok, can not attach html file to this replay

the only difference that i can see is a blank line because of the new label, but it might be not at bad thing at all, but i can remove the label and reuse the existing one that is for the heading NA Values, that will be several lines above

jreback · 2017-06-08T13:26:15Z

can u show a screenshot of the built page & also of the doc-string (in ipython)

OlegShteynbuk · 2017-06-08T13:44:53Z

OlegShteynbuk · 2017-06-08T13:46:44Z

have added the label and reference to the label, not the doc-string (in ipython) probably will be the next step

jreback · 2017-06-09T10:28:20Z

lgtm. @TomAugspurger @jorisvandenbossche

TomAugspurger · 2017-06-09T18:29:49Z

doc/source/io.rst

-  NA values. By default the following values are interpreted as NaN:
-  ``'-1.#IND', '1.#QNAN', '1.#IND', '-1.#QNAN', '#N/A N/A', '#N/A', 'N/A', 'n/a', 'NA',
-  '#NA', 'NULL', 'null', 'NaN', '-NaN', 'nan', '-nan', ''``.
+  NA values. By default the following values are interpreted as NaN: See :ref:`na values const


The sentence "By default the following values are interpreted as NaN:" can be removed. I think

See :ref:`na values const <io.navaluesconst>` below for a list of the values interpreted as NaN by default.

…sers.pyx

jreback · 2017-06-13T23:18:28Z

thanks @OlegShteynbuk
have a look at the built docs (prob take a bit to generate) http://pandas-docs.github.io/pandas-docs-travis/

and if anything looks amiss, pls open a PR

OlegShteynbuk · 2017-06-15T15:57:48Z

@jreback built docs are OK, it really took some time to generate.

OlegShteynbuk mentioned this pull request Jun 3, 2017

make null lowercase a missing value #16534

Merged

jreback requested changes Jun 3, 2017

View reviewed changes

OlegShteynbuk force-pushed the na_values branch from cfb46d7 to c2cbdf9 Compare June 5, 2017 03:47

jreback approved these changes Jun 5, 2017

View reviewed changes

jreback added Clean IO CSV read_csv, to_csv Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate labels Jun 5, 2017

jreback added this to the 0.21.0 milestone Jun 5, 2017

OlegShteynbuk mentioned this pull request Jun 5, 2017

consolidate the duplicate definitions of NA values in code and docs (refactoring, cleaning) #16606

Closed

OlegShteynbuk force-pushed the na_values branch from 2fb1178 to 8c1d668 Compare June 7, 2017 22:26

jreback reviewed Jun 8, 2017

View reviewed changes

TomAugspurger approved these changes Jun 9, 2017

View reviewed changes

OlegShteynbuk force-pushed the na_values branch 2 times, most recently from db67d62 to d5a52ca Compare June 12, 2017 19:40

consolidated the duplicate definitions of NA values (in parsers & IO)

bc6752a

OlegShteynbuk added 13 commits June 13, 2017 07:46

consolidated the duplicate definitions of NA values in parsers.pyx

0650da0

added blank lines - E302 expected 2 blank lines, found 1 in parsers.pyx

b0c1d4d

added pesky blank lines - E302 expected 2 blank lines, found 1 in par…

1f0a350

…sers.pyx

added pesky blank lines - E302 expected 2 blank lines, found 1 in par…

0579460

…sers.pyx

changed to using existing function _ensure_encoded in parsers.pyx

6225c7c

reverted na_values.py back to using assert

a7e7f01

fixing typo

dc9a368

convert set to list

e8f6e82

put definition before reference

5770506

import _NA_VALUES from common instead of parsers, make import explicit

5fa829e

in import replace common with com

c499448

remove duplicate na values in in io.rst

03a335b

improved text related to na values in io.rst file

10cba2c

OlegShteynbuk force-pushed the na_values branch from d5a52ca to 10cba2c Compare June 13, 2017 11:48

jreback merged commit 3caf858 into pandas-dev:master Jun 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consolidated the duplicate definitions of NA values (in parsers & IO) #16589

consolidated the duplicate definitions of NA values (in parsers & IO) #16589

OlegShteynbuk commented Jun 3, 2017 •

edited by jreback

Loading

codecov bot commented Jun 3, 2017 •

edited

Loading

jreback Jun 3, 2017

OlegShteynbuk Jun 5, 2017

jreback Jun 3, 2017

OlegShteynbuk Jun 3, 2017 •

edited

Loading

OlegShteynbuk Jun 5, 2017

OlegShteynbuk Jun 5, 2017

jreback commented Jun 5, 2017

gfyoung commented Jun 5, 2017

OlegShteynbuk commented Jun 5, 2017 •

edited

Loading

jreback commented Jun 5, 2017

OlegShteynbuk commented Jun 5, 2017

gfyoung commented Jun 5, 2017 •

edited

Loading

OlegShteynbuk commented Jun 5, 2017

OlegShteynbuk commented Jun 5, 2017

jreback commented Jun 6, 2017

OlegShteynbuk commented Jun 7, 2017

jreback Jun 8, 2017

OlegShteynbuk Jun 8, 2017

OlegShteynbuk Jun 8, 2017

jreback commented Jun 8, 2017

OlegShteynbuk commented Jun 8, 2017

OlegShteynbuk commented Jun 8, 2017

jreback commented Jun 9, 2017

TomAugspurger Jun 9, 2017

jreback commented Jun 13, 2017

OlegShteynbuk commented Jun 15, 2017 •

edited

Loading

consolidated the duplicate definitions of NA values (in parsers & IO) #16589

consolidated the duplicate definitions of NA values (in parsers & IO) #16589

Conversation

OlegShteynbuk commented Jun 3, 2017 • edited by jreback Loading

codecov bot commented Jun 3, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlegShteynbuk Jun 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jun 5, 2017

gfyoung commented Jun 5, 2017

OlegShteynbuk commented Jun 5, 2017 • edited Loading

jreback commented Jun 5, 2017

OlegShteynbuk commented Jun 5, 2017

gfyoung commented Jun 5, 2017 • edited Loading

OlegShteynbuk commented Jun 5, 2017

OlegShteynbuk commented Jun 5, 2017

jreback commented Jun 6, 2017

OlegShteynbuk commented Jun 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jun 8, 2017

OlegShteynbuk commented Jun 8, 2017

OlegShteynbuk commented Jun 8, 2017

jreback commented Jun 9, 2017

Choose a reason for hiding this comment

jreback commented Jun 13, 2017

OlegShteynbuk commented Jun 15, 2017 • edited Loading

OlegShteynbuk commented Jun 3, 2017 •

edited by jreback

Loading

codecov bot commented Jun 3, 2017 •

edited

Loading

OlegShteynbuk Jun 3, 2017 •

edited

Loading

OlegShteynbuk commented Jun 5, 2017 •

edited

Loading

gfyoung commented Jun 5, 2017 •

edited

Loading

OlegShteynbuk commented Jun 15, 2017 •

edited

Loading