Revert "STYLE use pandas-dev-flaker (#40906) #50519
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
December 31, 2022 18:00
This reverts commit 1f27ed0.
MarcoGorelli
force-pushed
the
rip-out-pandas-dev-flaker-from-flake8
branch
from
2357226 to
a747171
Compare
MarcoGorelli
commented
Jan 2, 2023
| pytest.xfail("known failure on some windows platforms") | ||
| pytest.mark.xfail("known failure on some windows platforms") |
There was a problem hiding this comment.
the pytest.xfail check in pandas-dev-flaker only checks decorators. My bad here, a simple regex ended up being the better solution here
There was a problem hiding this comment.
I think this will make it appear that this test will pass when it hits this line as this mark is not attached to anything.
Probably needs something like
except OverflowError:
if is_platform_windows():
request.node.add_marker(pytest.mark.xfail(reason=...)
raise
added 3 commits
January 3, 2023 18:31
mroeschke
reviewed
Jan 3, 2023
| pytest.xfail("known failure on some windows platforms") | ||
| request.node.add_marker( | ||
| pytest.mark.xfail("known failure on some windows platforms") | ||
| ) | ||
| else: |
There was a problem hiding this comment.
Nit: I think this else should be removed now too so the xfail will be recognized after the raise
…flaker-from-flake8
…rcoGorelli/pandas into rip-out-pandas-dev-flaker-from-flake8
…flaker-from-flake8
|
thanks - merging to clear the queue then, the only failures are the sqlalchemy ones, which hopefully #50580 will address |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So, #40906 consolidated a bunch of custom checks into a flake8 extension
As it stands, flake8 is the slowest pre-commit hook we have (even without the extension - see timings)
In #50160, Joris tried adding ruff, which would more than an order of magnitude faster (!), and has more checks. Several projects are moving towards it
So, I suggest we revert #40906 , so that we can rip out flake8 and can replace it with
ruffwithout losing the custom checksThis is mostly a pure revert of #40906, with some adjustments for things which have since changed
I removed the
import pandas.core.common as comone, as that one wasnoqa'd in a few places anyway (if the check isn't a flake8 extension, then it's not so easy to turn off the check, we it's done for a whole file) and doesn't seem too important anywayOverall, I think that this + the ruff PR would be a big improvement to developer workflow. It would mean more hooks (for now, maybe then can be consolidated anyway), but they would run a lot faster. And I'd expect the speed of the checks to matter much more to developer workflow than the speed of them