DOC: Fix flake8 problems in io.rst #23855

saurav2608 · 2018-11-22T06:59:20Z

closes DOC: Fix format of io.rst #23791
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Pushing this code to check if this clears CI. Made a few changes towards issue #23791

codecov · 2018-11-22T07:41:31Z

Codecov Report

Merging #23855 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #23855   +/-   ##
=======================================
  Coverage   42.46%   42.46%           
=======================================
  Files         161      161           
  Lines       51557    51557           
=======================================
  Hits        21892    21892           
  Misses      29665    29665

Flag	Coverage Δ
#single	`42.46% <ø> (ø)`	⬆️

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 b45eb26...d8b555a. Read the comment docs.

datapythonista

lgtm, but there are couple of unrelated things, that if you can fix here, that would be great.

datapythonista · 2018-11-22T10:40:19Z

doc/source/io.rst

   import pandas as pd
+   import pandas.util.testing as tm
+   from pandas.compat import StringIO, BytesIO
   ExcelWriter = pd.ExcelWriter


It's somehow unrelated to your changes, but if you don't mind removing this line, and replacing ExcelWriter by pd.ExcelWriter wherever is used, that would be great. I think reassigning this just make things difficult for the users to understand.

And if import pandas.util.testing as tm is used just in one code block or section, would be nice to move it there, as it may not be obvious for users to know what tm is without the import. If it's used in many places leave it here, and we'll see what we can do later on.

datapythonista

Couple of comments

datapythonista · 2018-11-22T21:00:04Z

doc/source/io.rst

-      data = '# empty\n# second empty line\n# third empty' \
-                'line\nX,Y,Z\n1,2,3\nA,B,C\n1,2.,4.\n5.,NaN,10.0'
+      data = """# empty\n# second empty line\n# third empty \
+                line\nX,Y,Z\n1,2,3\nA,B,C\n1,2.,4.\n5.,NaN,10.0"""


those lines are not equivalent

I am unable to resolve an error here. On my local build the the second line is not appended to the variable data. I am quoting the code here. Need your help.
`
In [68]: data = '# empty\n# second empty line\n# third empty' \

In [69]: 'line\nX,Y,Z\n1,2,3\nA,B,C\n1,2.,4.\n5.,NaN,10.0'
Out[69]: 'line\nX,Y,Z\n1,2,3\nA,B,C\n1,2.,4.\n5.,NaN,10.0'

In [70]: print(data)
# empty
# second empty line
# third empty

In [71]: pd.read_csv(StringIO(data), comment='#', skiprows=4, header=1)

EmptyDataError Traceback (most recent call last)
in
----> 1 pd.read_csv(StringIO(data), comment='#', skiprows=4, header=1)
`

Note that if I copy paste these lines to a ipython terminal. It works well. I tried combination of spaces etc.

@datapythonista - Can you please confirm the above? Otherwise everything is ready for a commit and push.

Sorry, I missed that.

I didn't quite understand what's the problem, but checking it again, I think much better ways to write that are:

data = '\n'.join(['# empty', '# second empty line', '# third empty', 'line', 'X,Y,Z', '1,2,3', 'A,B,C', '1,2.,4.', '5.,NaN,10.0'])

or

data = '''# empty # second empty line # third empty line X,Y,Z 1,2,3 A,B,C 1,2.,4. 5.,NaN,10.0'''

which makes possible to read the content

datapythonista · 2018-11-22T21:02:13Z

doc/source/io.rst

   import numpy as np
-   np.random.seed(123456)
+   import pandas as pd
+   import pandas.io.date_converters as conv


I think this should be left where it was. Any reason to move it here?

or even better, I'd use pd..io.date_converters..., no extra import is needed, and the code is more explicit

datapythonista · 2018-11-22T21:02:50Z

doc/source/io.rst

   pd.options.display.max_rows = 15
   clipdf = pd.DataFrame({'A': [1, 2, 3], 'B': [4, 5, 6], 'C': ['p', 'q', 'r']},
                         index=['x', 'y', 'z'])

+
+


are those needed for the validation? I'd say flake8-rst should fail with 3 blank lines

datapythonista · 2018-11-22T21:03:51Z

doc/source/io.rst

@@ -718,7 +715,8 @@ result in byte strings being decoded to unicode in the result:

 .. ipython:: python

-   data = b'word,length\nTr\xc3\xa4umen,7\nGr\xc3\xbc\xc3\x9fe,5'.decode('utf8').encode('latin-1')
+   data = (b'word,length\nTr\xc3\xa4umen,7\nGr\xc3\xbc\xc3\x9fe,5'.
+           decode('utf8').encode('latin-1'))


I think it'll be more readable as:

data = b'...' data = data.decode(...

datapythonista · 2018-11-22T21:05:23Z

doc/source/io.rst

@@ -2186,6 +2179,7 @@ A few notes on the generated table schema:

  .. ipython:: python

+


I don't think this new blank line is correct

datapythonista · 2018-11-22T21:06:22Z

doc/source/io.rst


 .. ipython:: python

+


same, blank line not needed

datapythonista · 2018-11-22T21:10:35Z

@saurav2608 in general try to not open the pull requests until you're done. You can push to your remote branch, but if you open a PR, someone will review it, and it's a waste of time if your changes are not finished (unless you need a review to make sure what you're doing is correct...)

saurav2608 · 2018-11-23T02:47:49Z

@saurav2608 in general try to not open the pull requests until you're done.

@datapythonista - I will keep this in mind henceforth. I will edit the .rst file over the weekend.

datapythonista · 2018-11-26T10:10:12Z

@saurav2608 do you have time to address the comments?

saurav2608 · 2018-11-26T11:21:15Z

I got swamped by some other work. I can take this up on Wednesday.

saurav2608 · 2018-11-29T02:40:06Z

HI @datapythonista - travis fails with the below error.

[create env]
Solving environment: ...working... failed
ResolvePackageNotFound:

flake8-rst=0.4.2

I believe you have fixed this in #23975 . Is there a way to re-initiate the test?

datapythonista

I rerun travis. But reviewing this again, I saw that we're using different ways of creating multiline strings. If you don't mind, would be great to standardize them

datapythonista · 2018-11-29T10:15:40Z

doc/source/io.rst

-                'line\nX,Y,Z\n1,2,3\nA,B,C\n1,2.,4.\n5.,NaN,10.0'
-      print(data)
-      pd.read_csv(StringIO(data), comment='#', skiprows=4, header=1)
+   data = '\n'.join(['# empty',


sorry I didn't see it before, but looks like mostly everywhere on this file, what is being used is:

data = ('#empty\n' '#second empty line' ...

I think it would be nice to keep the same for consistency. Or was this the format that was failing in this case?

Yes it does. I wonder what I was thinking here. :(

datapythonista · 2018-11-29T10:16:07Z

doc/source/io.rst

@@ -718,7 +724,8 @@ result in byte strings being decoded to unicode in the result:

 .. ipython:: python

-   data = b'word,length\nTr\xc3\xa4umen,7\nGr\xc3\xbc\xc3\x9fe,5'.decode('utf8').encode('latin-1')
+   data = b'word,length\nTr\xc3\xa4umen,7\nGr\xc3\xbc\xc3\x9fe,5'


do you mind breaking this in lines in the way previously suggested?

datapythonista · 2018-11-29T10:16:19Z

doc/source/io.rst

@@ -992,7 +998,7 @@ DD/MM/YYYY instead. For convenience, a ``dayfirst`` keyword is provided:

   data = "date,value,cat\n1/6/2000,5,a\n2/6/2000,10,b\n3/6/2000,15,c"


datapythonista · 2018-11-29T10:16:42Z

doc/source/io.rst

@@ -1166,7 +1175,7 @@ options as follows:

 .. ipython:: python

-    data= 'a,b,c\n1,Yes,2\n3,No,4'
+    data = 'a,b,c\n1,Yes,2\n3,No,4'


any reason to not split this in several lines as in the rest?

jreback · 2018-11-29T17:23:12Z

can you rebase

saurav2608 · 2018-11-30T05:08:58Z

can you rebase

@jreback : Unfortunately, I am not able to. I messed up at some point and now rebase and squash generates conflict errors that I am not resolve it with my knowledge of git.

I am open to ideas on how to squash the commits. In the worst case, I can delete the branch and start again.

datapythonista · 2018-11-30T08:09:05Z

just do a git fetch upstream && git merge upstream/master edit the file with the conflict, keep the lines that arr ok, and leave the file how it has to be, do a git add of the file, commit and push

saurav2608 · 2018-11-30T12:02:17Z

I am not sure if this worked.

saurav2608 · 2018-11-30T12:05:27Z

I am not sure if this worked.

It did not. Somehow an old version of the file was pushed.

datapythonista · 2018-11-30T12:15:37Z

Can you take a look at the first section of: https://datapythonista.github.io/blog/useful-git-commands.html

saurav2608 · 2018-11-30T15:36:47Z

@datapythonista - I followed your post. But if I try to push I get below error. Should I pull one before pushing?
! [rejected] io-doc -> io-doc (non-fast-forward) error: failed to push some refs to 'https://github.com/saurav2608/pandas.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

saurav2608 · 2018-11-30T17:00:46Z

@datapythonista - I followed your post. But if I try to push I get below error. Should I pull one before pushing?
! [rejected] io-doc -> io-doc (non-fast-forward) error: failed to push some refs to 'https://github.com/saurav2608/pandas.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

This is done I think. I forced a push on my forked repo.

datapythonista

lgtm, thanks @saurav2608

datapythonista · 2018-12-01T00:32:28Z

@saurav2608 can you do in this PR branch: git fetch upstream && git merge upstream/master && git push please

I'm not sure why the CI is failing, I think it shouldn't, but hopefully updating the branch will fix it.

pep8speaks · 2018-12-01T01:44:22Z

Hello @saurav2608! Thanks for updating the PR.

There are no PEP8 issues in the file asv_bench/benchmarks/categoricals.py !
There are no PEP8 issues in the file pandas/core/arrays/categorical.py !
There are no PEP8 issues in the file pandas/core/indexes/category.py !
There are no PEP8 issues in the file pandas/core/reshape/tile.py !
There are no PEP8 issues in the file pandas/tests/reshape/test_tile.py !
There are no PEP8 issues in the file pandas/tests/util/test_hashing.py !

datapythonista · 2018-12-01T01:59:24Z

@saurav2608 looks like the PR contains unrelated changes again. Not sure what is causing this, I don't think the command I wrote causes it. But in any case, can you repeat what you did before to fix the history and leave only your changes in the PR please.

saurav2608 · 2018-12-01T16:12:15Z

I redid the PR. this look okay now. Please review once and let me know.

datapythonista

thanks @saurav2608, just couple of small things if you don't mind

datapythonista · 2018-12-01T23:42:26Z

doc/source/io.rst

@@ -1166,7 +1175,7 @@ options as follows:

 .. ipython:: python

-    data= 'a,b,c\n1,Yes,2\n3,No,4'
+    data = 'a,b,c\n1,Yes,2\n3,No,4'


any reason to not split this in several lines as in the rest?

datapythonista · 2018-12-01T23:43:56Z

doc/source/io.rst

@@ -2186,6 +2237,7 @@ A few notes on the generated table schema:

  .. ipython:: python

+


I guess this is giving an error, but that will be fixed in flake8-rst, as the error is wrong. So just one blank line please.

datapythonista

lgtm

thanks a lot for fixing all these @saurav2608

saurav2608 · 2018-12-02T12:05:39Z

@datapythonista - no problem. It took long for me to fix this. Thank for your guidance. There are still some issues to resolve. For example suppressing E402 module level import not at top of file type errors where the import is deliberately kept at that location. I can work on those once these changes are merged.

datapythonista · 2018-12-02T12:19:43Z

No worries @saurav2608, we all do what we can, and it also takes time for us to review.

I don't think we want to fix the E402, but make flake8-rst not report them as errors. I think it's better for users to have the imports in the block where they are used.

And don't worry much if not everything is fixed. There are so many things to fix at this point, that any fixes are very useful. Once most of the stuff is fixed, then we'll check more in detail the outstanding problems.

If you're looking for other things to contribute, it's probably better to wait until #23847 is merged to work on the rest of the .rst files, as it's a bit difficult to follow right now what is done, what's in progress, and what needs to be done. After that PR is merged, I'll create issues for the pages that need fixes, and it'll be easier.

One related issue that can be address and would be very useful to have implemented soon would be #23952.

jreback · 2018-12-02T18:14:04Z

@saurav2608 can you rebase and remove io.rst from the setup.cfg

datapythonista · 2018-12-02T18:16:04Z

@jreback if you want to merge this, I'll open a PR with the fixed files, and I'll add this one too.

jreback · 2018-12-02T18:18:37Z

thanks @saurav2608

@datapythonista go for it

datapythonista · 2018-12-02T18:21:48Z

Opened #24051, will ping you when is green.

datapythonista approved these changes Nov 22, 2018

View reviewed changes

datapythonista requested changes Nov 22, 2018

View reviewed changes

datapythonista changed the title ~~DOC: Reordered import to conform to PEP8 standards.~~ DOC: Fix flake8 problems in io.rst Nov 22, 2018

gfyoung added Docs Code Style Code style, linting, code_checks labels Nov 22, 2018

datapythonista reviewed Nov 29, 2018

View reviewed changes

saurav2608 force-pushed the io-doc branch from 863fea0 to 5faff0d Compare November 30, 2018 16:55

datapythonista approved these changes Nov 30, 2018

View reviewed changes

saurav2608 force-pushed the io-doc branch from aaa6b77 to a71cea3 Compare December 1, 2018 06:37

datapythonista reviewed Dec 1, 2018

View reviewed changes

DOC: conform to PEP-8

d8b555a

saurav2608 force-pushed the io-doc branch from 31027f2 to d8b555a Compare December 2, 2018 07:15

datapythonista approved these changes Dec 2, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Dec 2, 2018

jreback merged commit 6dd130a into pandas-dev:master Dec 2, 2018

saurav2608 deleted the io-doc branch December 3, 2018 02:52

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: conform to PEP-8 (pandas-dev#23855)

e02968a

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: conform to PEP-8 (pandas-dev#23855)

8d66f2b

		@@ -2186,6 +2179,7 @@ A few notes on the generated table schema:

		.. ipython:: python

		@@ -992,7 +998,7 @@ DD/MM/YYYY instead. For convenience, a ``dayfirst`` keyword is provided:

		data = "date,value,cat\n1/6/2000,5,a\n2/6/2000,10,b\n3/6/2000,15,c"

		@@ -2186,6 +2237,7 @@ A few notes on the generated table schema:

		.. ipython:: python

DOC: Fix flake8 problems in io.rst #23855

DOC: Fix flake8 problems in io.rst #23855

Conversation

saurav2608 commented Nov 22, 2018 • edited Loading

codecov bot commented Nov 22, 2018 • edited Loading

Codecov Report

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saurav2608 Nov 27, 2018 • edited Loading

Choose a reason for hiding this comment

In [71]: pd.read_csv(StringIO(data), comment='#', skiprows=4, header=1)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Nov 22, 2018

saurav2608 commented Nov 23, 2018

datapythonista commented Nov 26, 2018

saurav2608 commented Nov 26, 2018

saurav2608 commented Nov 29, 2018 • edited Loading

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Nov 29, 2018

saurav2608 commented Nov 30, 2018 • edited Loading

datapythonista commented Nov 30, 2018

saurav2608 commented Nov 30, 2018

saurav2608 commented Nov 30, 2018

datapythonista commented Nov 30, 2018

saurav2608 commented Nov 30, 2018

saurav2608 commented Nov 30, 2018

datapythonista left a comment

Choose a reason for hiding this comment

datapythonista commented Dec 1, 2018

pep8speaks commented Dec 1, 2018

datapythonista commented Dec 1, 2018

saurav2608 commented Dec 1, 2018

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista left a comment

Choose a reason for hiding this comment

saurav2608 commented Dec 2, 2018 • edited Loading

datapythonista commented Dec 2, 2018

jreback commented Dec 2, 2018

datapythonista commented Dec 2, 2018

jreback commented Dec 2, 2018

datapythonista commented Dec 2, 2018

saurav2608 commented Nov 22, 2018 •

edited

Loading

codecov bot commented Nov 22, 2018 •

edited

Loading

saurav2608 Nov 27, 2018 •

edited

Loading

saurav2608 commented Nov 29, 2018 •

edited

Loading

saurav2608 commented Nov 30, 2018 •

edited

Loading

saurav2608 commented Dec 2, 2018 •

edited

Loading