Skip to content

DOC: Fix isort output in CI #30974

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

Closed
datapythonista opened this issue Jan 13, 2020 · 6 comments · Fixed by #31733
Closed

DOC: Fix isort output in CI #30974

datapythonista opened this issue Jan 13, 2020 · 6 comments · Fixed by #31733
Labels
CI Continuous Integration good first issue

Comments

@datapythonista
Copy link
Member

In the CI, we're using isort to check whether the imports in Python files are sorted in an standard way. See https://github.com/pandas-dev/pandas/blob/master/ci/code_checks.sh#L114

The way it is implemented, we expect isort to not print anything in the terminal if everything is ok, and to show errors for the errors encountered. This is mostly true, except that isort is also reporting when there are files being skipped.

The result of this, is that we highlight in the CI as an error when there are files being skipped. See https://github.com/pandas-dev/pandas/pull/30946/checks?check_run_id=385833706#step:6:36

This can be misleading, and we should try to avoid it. Without adding much complexity to the already complex CI. Not sure if there is an isort parameter to simply not report the skipped files. Or if there is an easy way to not highlight the skipped files as an error.

@datapythonista datapythonista added CI Continuous Integration good first issue labels Jan 13, 2020
@souvik3333
Copy link
Contributor

There are no already-existing settings in isort for this. Can we not just catch the message from isort and if it's in Skipped * files format then ignore it? Also, this skip message is only appearing when all imports are sorted not when imports are not sorted.

@datapythonista
Copy link
Member Author

Ignoring that message sounds good, or any other solution. The only constraint is that we surely don't want to make that CI part much more complex. I'd prefer to just not have the ##[error] highlighting, than call isort in an unreadable or unreliable way.

@ndmar
Copy link

ndmar commented Jan 15, 2020

I wouldn't mind giving this a go. From a very cursory double check on this, it does indeed appear that the most straightforward solution would be to catch the stdout from the command, and ignore when the output matches the undesired message.

@leandermaben
Copy link
Contributor

Hey
I'd like to work on this issue

@d1618033
Copy link
Contributor

d1618033 commented Feb 5, 2020

The undocumented --quiet option in isort seems to do the trick.

@datapythonista
Copy link
Member Author

Excellent, thanks @d1618033 Do you want to open a pull request?

d1618033 added a commit to d1618033/pandas that referenced this issue Feb 6, 2020
d1618033 added a commit to d1618033/pandas that referenced this issue Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration good first issue
Projects
None yet
5 participants