Partialy fix issue #23334 - isort pandas/core/dtypes directory #23336

alexander-ponomaroff · 2018-10-25T20:57:52Z

The imports have been sorted with isort in the pandas/core/dtypes directory.

Moving onto sorting the pandas/core/groupby directory.

partly Fix import formatting using isort #23334
passes git diff upstream/master -u -- "*.py" | flake8 --diff

pep8speaks · 2018-10-25T20:57:57Z

Hello @alexander-ponomaroff! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/dtypes/api.py !
There are no PEP8 issues in the file pandas/core/dtypes/base.py !
There are no PEP8 issues in the file pandas/core/dtypes/cast.py !
There are no PEP8 issues in the file pandas/core/dtypes/common.py !
There are no PEP8 issues in the file pandas/core/dtypes/concat.py !
There are no PEP8 issues in the file pandas/core/dtypes/dtypes.py !
There are no PEP8 issues in the file pandas/core/dtypes/inference.py !
There are no PEP8 issues in the file pandas/core/dtypes/missing.py !

Comment last updated on October 25, 2018 at 21:04 Hours UTC

alimcmaster1 · 2018-10-25T21:12:50Z

Thanks @alexander-ponomaroff ! Lets wait for CI to run then i'm happy to review this.

alexander-ponomaroff · 2018-10-25T21:16:35Z

Thank you @alimcmaster1 . Could you please explain what "passes git diff upstream/master -u -- "*.py" | flake8 --diff" means? I noticed that you have this checked in your pull requests.

alimcmaster1 · 2018-10-25T21:26:44Z

Hey - please see the "Python (PEP8)" section in the contributing guide and further info on flake8 here - hope that helps.

alexander-ponomaroff · 2018-10-25T21:27:48Z

@alimcmaster1 How long does the CI usually run for? I would like to wait for the dtypes directory to be all good before starting to work on the groupby directory, but might need to leave soon for a couple hours. So I was wondering it the checks will finish before I have to leave.

alexander-ponomaroff · 2018-10-25T21:31:33Z

Hey - please see the "Python (PEP8)" section in the contributing guide and further info on flake8 here - hope that helps.

Thanks, just performed the check locally. But I'm assuming the pep8speaks bot, which is the second comment of this pr does the same thing here?

alimcmaster1 · 2018-10-25T21:34:41Z

https://travis-ci.org/pandas-dev/pandas/pull_requests , you can look at the CI run off the back of previous PRs to get an idea of how long it will take, feel free to wait till tomorrow there is no rush

codecov · 2018-10-25T23:07:59Z

Codecov Report

❗ No coverage uploaded for pull request base (master@0d6cb3c). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #23336   +/-   ##
=========================================
  Coverage          ?   92.18%           
=========================================
  Files             ?      161           
  Lines             ?    51186           
  Branches          ?        0           
=========================================
  Hits              ?    47188           
  Misses            ?     3998           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.62% <100%> (?)`
#single	`42.22% <100%> (?)`

Impacted Files	Coverage Δ
pandas/core/dtypes/base.py	`100% <100%> (ø)`
pandas/core/dtypes/concat.py	`96.34% <100%> (ø)`
pandas/core/dtypes/missing.py	`93.1% <100%> (ø)`
pandas/core/dtypes/dtypes.py	`95.61% <100%> (ø)`
pandas/core/dtypes/inference.py	`98.38% <100%> (ø)`
pandas/core/dtypes/cast.py	`88.92% <100%> (ø)`
pandas/core/dtypes/common.py	`94.37% <100%> (ø)`
pandas/core/dtypes/api.py	`100% <100%> (ø)`

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 0d6cb3c...a74f2b6. Read the comment docs.

jreback · 2018-10-26T00:03:29Z

pls merge master and ping on green.

alimcmaster1 · 2018-10-26T13:10:36Z

from pandas.core.dtypes.generic import ABCPeriodArray in common.py it looks like you have removed this import? Did isort do this?

alexander-ponomaroff · 2018-10-26T17:14:47Z

@alimcmaster1 Not sure what happened, I did not remove it on purpose.

alexander-ponomaroff · 2018-10-26T17:32:10Z

@alimcmaster1 I just added ABCPeriodArray back and fixed another conflict.

thoo · 2018-10-26T20:28:59Z

@alexander-ponomaroff You might want to rerun isort to fix travis CI. I did on some of my PRs and they are passed now. It might be due to upstream/merge.

alexander-ponomaroff · 2018-10-26T22:43:26Z

@thoo I reran isort, but the CI is still failing. @alimcmaster1 Do you have any suggestions on what I should do?

thoo · 2018-10-26T22:49:03Z

You can go to the log file and see what goes wrong. For example, this is the first one failed.

�[31m�[1m_____________ TestDataFrameCombineFirst.test_combine_first_period ______________�[0m
[gw1] linux -- Python 3.7.1 /home/travis/miniconda3/envs/pandas/bin/python

self = <pandas.tests.frame.test_combine_concat.TestDataFrameCombineFirst object at 0x7f06cdf74470>

�[1m    def test_combine_first_period(self):�[0m
�[1m        data1 = pd.PeriodIndex(['2011-01', 'NaT', '2011-03',�[0m
�[1m                                '2011-04'], freq='M')�[0m
�[1m        df1 = pd.DataFrame({'P': data1}, index=[1, 3, 5, 7])�[0m
�[1m        data2 = pd.PeriodIndex(['2012-01-01', '2012-02',�[0m
�[1m                                '2012-03'], freq='M')�[0m
�[1m        df2 = pd.DataFrame({'P': data2}, index=[2, 4, 5])�[0m
�[1m    �[0m
�[1m        res = df1.combine_first(df2)�[0m
�[1m        exp_dts = pd.PeriodIndex(['2011-01', '2012-01', 'NaT',�[0m
�[1m                                  '2012-02', '2011-03', '2011-04'],�[0m
�[1m                                 freq='M')�[0m
�[1m        exp = pd.DataFrame({'P': exp_dts}, index=[1, 2, 3, 4, 5, 7])�[0m
�[1m>       tm.assert_frame_equal(res, exp)�[0m
�[1m�[31mE       AssertionError: Attributes are different�[0m
�[1m�[31mE       �[0m
�[1m�[31mE       Attribute "dtype" are different�[0m
�[1m�[31mE       [left]:  int64�[0m
�[1m�[31mE       [right]: period[M]�[0m

�[1m�[31mpandas/tests/frame/test_combine_concat.py�[0m:775: AssertionError

thoo · 2018-10-26T22:51:04Z

I might rebase :

git fetch upstream
git merge upstream/master

and after that rerun isort to make sure everything is as you wanted. Rebase shouldn't change but it won't hurt.

alexander-ponomaroff · 2018-10-27T00:32:28Z

@jreback Hello Jeff, I cannot seem to identify the cause of CI failing, it recently started failing after I resolved the second conflict with the master branch.

alexander-ponomaroff · 2018-10-27T17:02:47Z

I notice that this was changed a few days ago, and it's now failing here:
https://github.com/pandas-dev/pandas/blame/5d84bc08ea7c45b0d2b3aa021a425b11531336f1/pandas/tests/frame/test_combine_concat.py#L776

@TomAugspurger you worked on this, do you have any advice for me? Or know why this failure was triggered in my pr?

alexander-ponomaroff · 2018-10-29T17:15:20Z

@alimcmaster1 I reran everything from scratch and the previously triggered error doesn't trigger anymore. However there is an error now KeyError: <class 'pandas._libs.tslibs.timestamps.Timestamp'>, not sure what this has to do with anything I did. Do you have any suggestions?

TomAugspurger · 2018-10-29T18:46:47Z

You may need to rebuild the C extensions: `python setup.py build_ext --inplace -j 4`

…

On Mon, Oct 29, 2018 at 12:19 PM Alexander Ponomaroff < ***@***.***> wrote: @alimcmaster1 <https://github.com/alimcmaster1> I reran everything from scratch and the previously triggered error doesn't trigger anymore. However there is an error now KeyError: <class 'pandas._libs.tslibs.timestamps.Timestamp'>, not sure what this has to do with anything I did. Do you have any suggestions? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23336 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIr_XmDwPuv7hwKzvSdVrOpVI6ZVRks5upziwgaJpZM4X7GYN> .

alexander-ponomaroff · 2018-10-29T20:50:49Z

@TomAugspurger I get this error: pandas/_libs/window.cpp:612:10: fatal error: 'ios' file not found

TomAugspurger · 2018-10-29T20:56:06Z

Others have reported that. What version of MacOS? You may need to update your command line tools.

…

On Mon, Oct 29, 2018 at 3:50 PM Alexander Ponomaroff < ***@***.***> wrote: @TomAugspurger <https://github.com/TomAugspurger> I get this error: pandas/_libs/window.cpp:612:10: fatal error: 'ios' file not found — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23336 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIj-voYoQqXrfJSYo5Wl_6dQa34mnks5up2oxgaJpZM4X7GYN> .

alexander-ponomaroff · 2018-10-29T20:57:33Z

@TomAugspurger High Sierra 10.13.6

TomAugspurger · 2018-10-29T21:00:38Z

What does `xcode-select --install` output?

…

On Mon, Oct 29, 2018 at 3:57 PM Alexander Ponomaroff < ***@***.***> wrote: @TomAugspurger <https://github.com/TomAugspurger> High Sierra 10.13.6 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23336 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIleq6JpOfNbLbcas2LrfbEkzB3AAks5up2vPgaJpZM4X7GYN> .

alexander-ponomaroff · 2018-10-29T21:07:17Z

@TomAugspurger Got it, I updated the command line tools and it worked. I rebuilt the C extensions. Where do I go from here?

alexander-ponomaroff · 2018-10-29T21:15:06Z

@TomAugspurger Btw, the error KeyError: <class 'pandas._libs.tslibs.timestamps.Timestamp'> is in one of the tests from Azure Pipelines, in case you thought that it was local. I just noticed that you're replying from an email. That's the only check I fail from the CI checks.

alimcmaster1 · 2018-10-29T23:32:36Z

pandas/core/dtypes/concat.py

-Utility functions related to concat
-"""
+    Utility functions related to concat
+    """


Don't think this indenting is required?

Will fix, accidental.

alimcmaster1 · 2018-10-30T00:09:57Z

"Btw, the error KeyError: <class 'pandas._libs.tslibs.timestamps.Timestamp'> is in one of the tests from Azure Pipelines"

I don't think the pipeline is failing because of this. Here is an example of a pipeline with the same test errors but the pipeline succeeds.

Your pipeline seems to be failing "WindowsPy27 py36_np121" builds with the error:

2018-10-29T15:30:51.4397165Z Could not find conda environment: pandas
2018-10-29T15:30:51.4398019Z You can list all discoverable environments with conda info --envs.
2018-10-29T15:30:56.1307704Z ModuleNotFoundError: No module named 'numpy'

Maybe @TomAugspurger could restart that for you? As i'm really not sure what the issue is there.

^ Ignore that @alexander-ponomaroff has just pushed a change- lets see if the above repeats.

alexander-ponomaroff · 2018-10-30T00:23:03Z

@alimcmaster1 Thank you very much. Hopefully nothing will fail this time.

Looks like isort on the dtype directory keeps triggering weird behaviours :).

alexander-ponomaroff · 2018-10-30T01:45:50Z

@alimcmaster1 Wow, finally! Everything passed. Thanks for you help.

alexander-ponomaroff · 2018-10-30T01:46:42Z

@jreback Everything passed now, could you please merge if everything is good?

jreback · 2018-10-30T11:37:40Z

pandas/core/dtypes/missing.py

@@ -1,28 +1,20 @@
 """
-missing types & inference
-"""


why did this change?

Accidental, will fix. When I had errors, I copy pasted imports from master and reran isort to attempt to fix the errors and the spacing on some of the comments messed up.

jreback · 2018-10-30T11:38:00Z

pandas/core/dtypes/api.py

-                     is_list_like,
-                     is_hashable,
-                     is_named_tuple)
+# categorical; interval; datetimelike; string-like; sparse; numeric types; like


i am not sure of the value of this comment

This comment was generated by isort, will delete.

alexander-ponomaroff · 2018-10-30T15:19:26Z

@jreback The requested changes have been changed, all checks pass.

jreback · 2018-10-31T13:04:54Z

thanks @alexander-ponomaroff

…ory (pandas-dev#23336)

Partialy fix issue pandas-dev#23334 - isort pandas/core/dtypes directory

1ee7d42

Fix Line 5:80: E501 line too long (102 > 79 characters) issue

f1160c1

jreback added the Code Style Code style, linting, code_checks label Oct 26, 2018

jreback added this to the 0.24.0 milestone Oct 26, 2018

Merge branch 'master' into issue-23334

caf3a44

alexander-ponomaroff and others added 3 commits October 26, 2018 13:22

Added ABCPeriodArray back to common.py

7615a7a

merging

c7466ba

Merge branch 'master' into issue-23334

0473017

Rerun isort

30b6f95

merge

087f348

TomAugspurger mentioned this pull request Oct 27, 2018

TST: unxfail combine-concat #23383

Closed

Resolve conflict

f88cc15

Rerun isort, resolve conflict, rebase

ff2eaa5

alimcmaster1 reviewed Oct 29, 2018

View reviewed changes

Indentation fix

8172b67

jreback requested changes Oct 30, 2018

View reviewed changes

Requested changes

a74f2b6

jreback approved these changes Oct 31, 2018

View reviewed changes

jreback merged commit de39bfc into pandas-dev:master Oct 31, 2018

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

Partialy fix issue pandas-dev#23334 - isort pandas/core/dtypes direct…

1d07c94

…ory (pandas-dev#23336)

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

Partialy fix issue pandas-dev#23334 - isort pandas/core/dtypes direct…

c59f5eb

…ory (pandas-dev#23336)

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

Partialy fix issue pandas-dev#23334 - isort pandas/core/dtypes direct…

fea2e40

…ory (pandas-dev#23336)

Partialy fix issue #23334 - isort pandas/core/dtypes directory #23336

Partialy fix issue #23334 - isort pandas/core/dtypes directory #23336

Conversation

alexander-ponomaroff commented Oct 25, 2018 • edited by jreback Loading

pep8speaks commented Oct 25, 2018 • edited Loading

Comment last updated on October 25, 2018 at 21:04 Hours UTC

alimcmaster1 commented Oct 25, 2018

alexander-ponomaroff commented Oct 25, 2018

alimcmaster1 commented Oct 25, 2018

alexander-ponomaroff commented Oct 25, 2018

alexander-ponomaroff commented Oct 25, 2018

alimcmaster1 commented Oct 25, 2018

codecov bot commented Oct 25, 2018 • edited Loading

Codecov Report

jreback commented Oct 26, 2018

alimcmaster1 commented Oct 26, 2018

alexander-ponomaroff commented Oct 26, 2018

alexander-ponomaroff commented Oct 26, 2018

thoo commented Oct 26, 2018

alexander-ponomaroff commented Oct 26, 2018

thoo commented Oct 26, 2018 • edited Loading

thoo commented Oct 26, 2018 • edited Loading

alexander-ponomaroff commented Oct 27, 2018

alexander-ponomaroff commented Oct 27, 2018

alexander-ponomaroff commented Oct 29, 2018

TomAugspurger commented Oct 29, 2018 via email

alexander-ponomaroff commented Oct 29, 2018

TomAugspurger commented Oct 29, 2018 via email

alexander-ponomaroff commented Oct 29, 2018

TomAugspurger commented Oct 29, 2018 via email

alexander-ponomaroff commented Oct 29, 2018

alexander-ponomaroff commented Oct 29, 2018

alimcmaster1 Oct 29, 2018

Choose a reason for hiding this comment

alexander-ponomaroff Oct 29, 2018

Choose a reason for hiding this comment

alimcmaster1 commented Oct 30, 2018 • edited Loading

alexander-ponomaroff commented Oct 30, 2018 • edited Loading

alexander-ponomaroff commented Oct 30, 2018

alexander-ponomaroff commented Oct 30, 2018

jreback Oct 30, 2018

Choose a reason for hiding this comment

alexander-ponomaroff Oct 30, 2018

Choose a reason for hiding this comment

jreback Oct 30, 2018

Choose a reason for hiding this comment

alexander-ponomaroff Oct 30, 2018

Choose a reason for hiding this comment

alexander-ponomaroff commented Oct 30, 2018

jreback commented Oct 31, 2018

alexander-ponomaroff commented Oct 25, 2018 •

edited by jreback

Loading

pep8speaks commented Oct 25, 2018 •

edited

Loading

codecov bot commented Oct 25, 2018 •

edited

Loading

thoo commented Oct 26, 2018 •

edited

Loading

thoo commented Oct 26, 2018 •

edited

Loading

alimcmaster1 commented Oct 30, 2018 •

edited

Loading

alexander-ponomaroff commented Oct 30, 2018 •

edited

Loading