-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
DOC: more consistent flake8-commands in contributing.rst #23724
Conversation
Codecov Report
@@ Coverage Diff @@
## master #23724 +/- ##
=======================================
Coverage 92.25% 92.25%
=======================================
Files 161 161
Lines 51383 51383
=======================================
Hits 47404 47404
Misses 3979 3979
Continue to review full report at Codecov.
|
doc/source/contributing.rst
Outdated
@@ -575,7 +575,7 @@ the `flake8 <https://pypi.org/project/flake8>`_ tool | |||
and report any stylistic errors in your code. Therefore, it is helpful before | |||
submitting code to run the check yourself on the diff:: | |||
|
|||
git diff master -u -- "*.py" | flake8 --diff | |||
git diff upstream/master -u -- "pandas/*.py" | flake8 --diff |
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 "pandas/*.py"
? I think we should also check .py
files in scripts/
, ci/
and everywhere else.
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.
@datapythonista
Happy to remove that too, if desired. As the PR stands, this was another inconsistency, and all the other commands (i.a. also the raison-d'être for grep
) are filtering to pandas/
.
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 it's relevant here but want to call out that there can certainly be performance implications to not restricting queries to the pandas
directory. For instance, when grepping from the top level if you don't restrict to that folder you could have a command that runs potentially much longer if it starts searching through the asv_bench
environments
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.
True, but it's only checking the diff anyway, which - even in the asv_bench
- will be relatively small (or, for most PRs, non-existent)
Failure is due to #23726 |
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.
Rather than try to continually harmonize these can't we update the make rule and reference that instead?
AFAICT, the To me, these commands make sense to have, and it's not so much a continual harmonization, as a one-off (partly, but not only due to #23707, where I had focused more on just the windows side). |
I'm not referring to Line 15 in 1250500
|
@WillAyd, well, that part isn't windows-compatible (at least not out-of-the-box) - no |
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 - @datapythonista
Failure is just the rank segfault |
thanks! |
…fixed * upstream/master: DOC: more consistent flake8-commands in contributing.rst (pandas-dev#23724) DOC: Fixed the doctsring for _set_axis_name (GH 22895) (pandas-dev#22969) DOC: Improve GL03 message re: blank lines at end of docstrings. (pandas-dev#23649) TST: add tests for keeping dtype in Series.update (pandas-dev#23604) TST: For GH4861, Period and datetime in multiindex (pandas-dev#23776) TST: move .str-test to strings.py & parametrize it; precursor to pandas-dev#23582 (pandas-dev#23777) STY: isort tests/scalar, tests/tslibs, import libwindow instead of _window (pandas-dev#23787) BUG: fixed .str.contains(..., na=False) for categorical series (pandas-dev#22170) BUG: Maintain column order with groupby.nth (pandas-dev#22811) API/DEPR: replace kwarg "pat" with "sep" in str.[r]partition (pandas-dev#23767) CLN: Finish isort core (pandas-dev#23765) TST: Mark test_pct_max_many_rows with pytest.mark.single (pandas-dev#23799)
After #23707, I was looking at the rendered results in https://pandas-docs.github.io/pandas-docs-travis/contributing.html#id54, and noticed some inconsistencies:
git diff master
as opposed togit diff upstream/master
; this is also in contrast to what's in the PR template.py
(because the command was getting long)git
, which makes it redundant to pipe togrep
.This PR proposes to harmonize those things. Sorry I overlooked them in #23707.