Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CI: Address linting errors in flake8 >= 3.8.1 #34152

Merged
merged 6 commits into from
May 13, 2020
Merged

CI: Address linting errors in flake8 >= 3.8.1 #34152

merged 6 commits into from
May 13, 2020

Conversation

mgmarino
Copy link
Contributor

@mgmarino mgmarino commented May 13, 2020 •
edited
Loading

xref #34150

This partially addresses the above issue by:

  • Ignoring E741
  • Fixing other linting errors

The issue with flake8-rst is handled temporarily in #34151, and the build here will fail the flake8-rst part until that is merged in.

@mgmarino mgmarino marked this pull request as ready for review May 13, 2020 08:52
jreback
jreback requested changes May 13, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also merge master

pandas/core/internals/concat.py Outdated
@@ -443,7 +443,10 @@ def _is_uniform_join_units(join_units: List[JoinUnit]) -> bool:
# cannot necessarily join
return (
# all blocks need to have the same type
all(type(ju.block) is type(join_units[0].block) for ju in join_units)
all(
type(ju.block) is type(join_units[0].block) # noqa: E721
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead use instance

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I wasn't sure in this in this case if the exact type was actually important. Will update.

@mgmarino mgmarino force-pushed the handle-flaking-errors branch from d615434 to 61b87cb Compare May 13, 2020 12:32
@jreback jreback added the Code Style Code style, linting, code_checks label May 13, 2020
@mgmarino mgmarino force-pushed the handle-flaking-errors branch 2 times, most recently from b3c74b5 to 387fe1f Compare May 13, 2020 12:41
@mgmarino mgmarino force-pushed the handle-flaking-errors branch from 387fe1f to 49a038c Compare May 13, 2020 12:58
@jreback
Copy link
Contributor

jreback commented May 13, 2020

great, does this complete the flake issues? e.g. can we unpin after this one?

@jreback jreback added this to the 1.1 milestone May 13, 2020
@mgmarino
Copy link
Contributor Author

mgmarino commented May 13, 2020 •
edited
Loading

great, does this complete the flake issues? e.g. can we unpin after this one?

It fixes all the issues with flake8 in the code, but it unfortunately doesn't fix the issue with flake8-rst flake8-docs/flake8-rst#22. That's a problem with the flake8-rst tool itself.

So, no, until flake-rst is fixed, we can't unpin.

@jreback jreback merged commit f9809fe into pandas-dev:master May 13, 2020
@jreback
Copy link
Contributor

jreback commented May 13, 2020

ok thanks.

@mgmarino mgmarino deleted the handle-flaking-errors branch May 27, 2020 08:23
@simonjayhawkins
Copy link
Member

@meeseeksdev backport to 1.0.x

@lumberbot-app
Copy link

lumberbot-app bot commented Jun 15, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout 1.0.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 f9809fe9f206c96bda1e4be09ae17689fcd13adf
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #34152: CI: Address linting errors in flake8 >= 3.8.1'
  1. Push to a named branch :
git push YOURFORK 1.0.x:auto-backport-of-pr-34152-on-1.0.x
  1. Create a PR against branch 1.0.x, I would have named this PR:

"Backport PR #34152 on branch 1.0.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@simonjayhawkins
Copy link
Member

@jorisvandenbossche will start backporting this or could pin flake8 on 1.0.x? lmk

simonjayhawkins pushed a commit to simonjayhawkins/pandas that referenced this pull request Jun 15, 2020
@mgmarino @simonjayhawkins
CI: Address linting errors in flake8 >= 3.8.1 (pandas-dev#34152)
f196e8d
simonjayhawkins added a commit to simonjayhawkins/pandas that referenced this pull request Jun 15, 2020
@simonjayhawkins
Revert "CI: Address linting errors in flake8 >= 3.8.1 (pandas-dev#34152…
32cc00e 
…)"

This reverts commit f196e8d.
jorisvandenbossche
jorisvandenbossche reviewed Jun 15, 2020
View reviewed changes
pandas/core/internals/concat.py
@@ -443,7 +443,7 @@ def _is_uniform_join_units(join_units: List[JoinUnit]) -> bool:
# cannot necessarily join
return (
# all blocks need to have the same type
all(type(ju.block) is type(join_units[0].block) for ju in join_units)
all(isinstance(ju.block, type(join_units[0].block)) for ju in join_units)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is actually not equivalent. We have both ExtensionBlock as other Block types that are subclasses from ExtensionBlock.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jorisvandenbossche Thanks, in the first iteration of the PR, I actually retained the exact type check and opted for a # noqa. Should this be a new issue? We could also require this behavior in a test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually just looking at this. In the meantime it also seems that the location where this _is_uniform_join_units is used isn't that strict anymore about the exact block types (due to #33486), so it should be fine as is.

@jorisvandenbossche
Copy link
Member

@jorisvandenbossche will start backporting this or could pin flake8 on 1.0.x? lmk

I would personally not care about backporting style related issues. Just pinning flake8 seems easier (or letting the lint build fail ..)

@simonjayhawkins
Copy link
Member

@jorisvandenbossche will start backporting this or could pin flake8 on 1.0.x? lmk

I would personally not care about backporting style related issues. Just pinning flake8 seems easier (or letting the lint build fail ..)

yep. not backporting this at this time.

@jorisvandenbossche jorisvandenbossche mentioned this pull request Sep 17, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.
4 participants
@mgmarino @jreback @simonjayhawkins @jorisvandenbossche