-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: Fix flake8 issues with whatsnew v0.18.* #24303
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
Conversation
Fixed almost every flake8 issues in v0.18.*.rst except one issue about tab-completion.
From conversations in pandas-dev#24273, I found that everywhere in pandas documentation should use `io` and not `compat`.
thanks @JimJeon : Please wait for a formal review from the maintainers. I just want to point that you have to remove the block between 8 & 11 for both the files (the below chunk should be deleted). Removal should generate some more errors.
|
also, for the tab error, we use a noqa, do a |
Codecov Report
@@ Coverage Diff @@
## master #24303 +/- ##
=======================================
Coverage 92.28% 92.28%
=======================================
Files 162 162
Lines 51827 51827
=======================================
Hits 47827 47827
Misses 4000 4000
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24303 +/- ##
==========================================
+ Coverage 92.28% 92.28% +<.01%
==========================================
Files 162 162
Lines 51827 51831 +4
==========================================
+ Hits 47827 47831 +4
Misses 4000 4000
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, added some comments, also what @saurav2608 said and the tab.
doc/source/whatsnew/v0.18.1.rst
Outdated
@@ -53,6 +53,7 @@ Friday before MLK Day | |||
|
|||
.. ipython:: python | |||
|
|||
from datetime import datetime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use import datetime
instead, and use datetime.datetime(...)
below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. May I ask why you prefer that?
Is that because it can cause some namespace issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are many reasons, and some are not trivial to explain.
The main one is that if I see a bare datetime
in Python code, I need to guess whether it's the main module, or the datetime.datetime
module. While there is no ambiguity in reading datetime.datetime
.
Then, with a from datetime import datetime
I load the whole datetime
module in sys.modules
, but I only have access in the namespace to the datetime.datetime
submodule (with the name datetime
). This won't let me perform operations like imp.reload(datetime)
, as I don't have anything in the namespace pointing to the main datetime
module.
doc/source/whatsnew/v0.18.1.rst
Outdated
freq='12H'), | ||
['a', 'b']])) | ||
index=pd.MultiIndex. | ||
from_product([pd.date_range('20130101', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this indentation confusing. If the lines are too long, can we use something like:
dft2 = pd.DataFrame(
np.random.randn(20, 1),
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. I wasn't flexible with that. I'll fix it.
doc/source/whatsnew/v0.18.1.rst
Outdated
@@ -451,15 +454,17 @@ New Behavior: | |||
.. code-block:: python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. code-block:: python | |
.. code-block:: ipython |
doc/source/whatsnew/v0.18.1.rst
Outdated
.. ipython:: python | ||
:suppress: | ||
|
||
from io import StringIO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use import io
instead, and have the import visible in the next block, where it's used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I'll fix it.
@saurav2608 @datapythonista Thanks for the reviews! |
doc/source/whatsnew/v0.18.1.rst
Outdated
.. ipython:: python | ||
:suppress: | ||
|
||
import io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you move the import to the beginning of the next code block, and remove this block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
In order to standardize the ``read_csv`` API for both the ``c`` and ``python`` engines, both will now raise an | ||
``EmptyDataError``, a subclass of ``ValueError``, in response to empty columns or header (:issue:`12493`, :issue:`12506`) | ||
|
||
Previous behaviour: | ||
|
||
.. code-block:: ipython | ||
In [1]: import io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need a blank line here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
some comments. ping on green. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks @JimJeon
thanks |
Fixed almost every flake8 issues in v0.18.*.rst except one issue
about tab-completion.
git diff upstream/master -u -- "*.py" | flake8 --diff
I fixed almost every flake8 issues except one.
An issue about tab-completion and I have no idea how to fix it.
In
doc/source/whatsnew/v0.18.0.rst
there is a code like below.I've searched with
grep
in every whatsnew files but there was only one other example about this inv0.17.0.rst
. Would you review this PR?