-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Partialy fix issue #23334 - isort pandas/core/dtypes directory #23336
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
Hello @alexander-ponomaroff! Thanks for updating the PR.
Comment last updated on October 25, 2018 at 21:04 Hours UTC |
Thanks @alexander-ponomaroff ! Lets wait for CI to run then i'm happy to review this. |
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. |
Hey - please see the "Python (PEP8)" section in the contributing guide and further info on flake8 here - hope that helps. |
@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. |
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? |
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 Report
@@ Coverage Diff @@
## master #23336 +/- ##
=========================================
Coverage ? 92.18%
=========================================
Files ? 161
Lines ? 51186
Branches ? 0
=========================================
Hits ? 47188
Misses ? 3998
Partials ? 0
Continue to review full report at Codecov.
|
pls merge master and ping on green. |
|
@alimcmaster1 Not sure what happened, I did not remove it on purpose. |
@alimcmaster1 I just added ABCPeriodArray back and fixed another conflict. |
@alexander-ponomaroff You might want to rerun |
@thoo I reran isort, but the CI is still failing. @alimcmaster1 Do you have any suggestions on what I should do? |
You can go to the log file and see what goes wrong. For example, this is the first one failed.
|
I might rebase :
and after that rerun |
@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. |
I notice that this was changed a few days ago, and it's now failing here: @TomAugspurger you worked on this, do you have any advice for me? Or know why this failure was triggered in my pr? |
@alimcmaster1 I reran everything from scratch and the previously triggered error doesn't trigger anymore. However there is an error now |
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>
.
|
@TomAugspurger I get this error: |
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>
.
|
@TomAugspurger High Sierra 10.13.6 |
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>
.
|
@TomAugspurger Got it, I updated the command line tools and it worked. I rebuilt the C extensions. Where do I go from here? |
@TomAugspurger Btw, the error |
pandas/core/dtypes/concat.py
Outdated
Utility functions related to concat | ||
""" | ||
Utility functions related to concat | ||
""" |
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.
Don't think this indenting is required?
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.
Will fix, accidental.
"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 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. |
@alimcmaster1 Thank you very much. Hopefully nothing will fail this time. Looks like isort on the dtype directory keeps triggering weird behaviours :). |
@alimcmaster1 Wow, finally! Everything passed. Thanks for you help. |
@jreback Everything passed now, could you please merge if everything is good? |
pandas/core/dtypes/missing.py
Outdated
@@ -1,28 +1,20 @@ | |||
""" | |||
missing types & inference | |||
""" |
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.
why did this change?
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.
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.
pandas/core/dtypes/api.py
Outdated
is_list_like, | ||
is_hashable, | ||
is_named_tuple) | ||
# categorical; interval; datetimelike; string-like; sparse; numeric types; like |
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 am not sure of the value of this comment
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.
This comment was generated by isort, will delete.
@jreback The requested changes have been changed, all checks pass. |
thanks @alexander-ponomaroff |
The imports have been sorted with isort in the pandas/core/dtypes directory.
Moving onto sorting the pandas/core/groupby directory.
git diff upstream/master -u -- "*.py" | flake8 --diff