CI: format isort output for detection by azure pipelines
State: closed

- [ ] closes #27179 
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: CI
description: Continuous Integration

Comments:
@datapythonista sorry I'm a bit new to bash scripting. It's definitely catching the import error but it is ignoring the exception as shown here: 

```Exception ignored in: <_io.TextIOWrapper name='<stdout>' mode='w' encoding='UTF-8'>
BrokenPipeError: [Errno 32] Broken pipe
Check import format using isort DONE
##[error]Bash exited with code '120'.```
pinging for review! Spent some time on it to understand `awk`. @datapythonista  thank you!
Seems like `awk` is keeping the exit status of isort, I guess it's because of the `set -o pipefail` in the function at the top of the function, we'll have to fix that.

But the first things would probably be fixing the output, from:
```
ERROR: /home/vsts/work/1/s/pandas/plotting/_matplotlib/core.py 
##[error]%(path)s(,): error : %(text)s
```
to
```
##vso[task.logissue type=error;sourcepath=/home/vsts/work/1/s/pandas/plotting/_matplotlib/core.py] Imports are incorrectly sorted.
```

Thanks!
ok! I think I'm getting closer now. @datapythonista could you check out the failure? I've pasted below what it's returning for ease.

```
Check import format using isort
##[error]pandas/plotting/_matplotlib/core.py(,): error :   Imports are incorrectly sorted.Check import format using isort DONE
##[error]Bash exited with code '1'.
##[section]Finishing: Linting
```

I added the ORS as the error message was moving to a new line with print. If there is a better way to do this do let me know, thank you!
is there a reason to not use the `##vso` code I suggested? I think is what we use in the other cases, if there is no reason better to keep consistent.
> is there a reason to not use the `##vso` code I suggested? I think is what we use in the other cases, if there is no reason better to keep consistent.

er, I didn't mean to remove anything. I'm not too familiar on vso codes but I read up on logging here: https://github.com/microsoft/azure-pipelines-tasks/blob/master/docs/authoring/commands.md

Could you clarify what you mean by the vso code and I will change it back, or point me to the docs. Thank you!
I refer to what is suggested in https://github.com/pandas-dev/pandas/pull/27334#issuecomment-515995494 instead of using `##[error]`
ah right. So oddly it seems to be getting converted from `##vso` to `##[error]` though the awk line I've used is `isort --recursive --check-only pandas asv_bench | awk '{ORS=""} {print "##vso[task.logissue type=error;sourcepath="$2";]"} {$1=$2=""; print $0}'`.

I might be missing something obvious here 
ah, sorry, didn't see in the code you were already using the `##vso`. I guess that's some azure magic then, but will be applied to all the other commands too, so it's ok.

I'd prefer to remove the `awk` and leave the original call when not using azure. Will be misleading if the errors are different when people call `isort` directly than with our script I think.

And the thing that concerns me is the `set -o pipefail` inside the if at the beginning. If eventually we change that code, your code will fail. And that doesn't seem right, since both things are unrelated, and shouldn't be using side-effects of each other. I'd start by adding `set +o pipefail` after the grep in line 39. That should already be there, will avoid the side effects, and will make your code fail. After that, you can wrap your code with the same `set -o pipefail` and `set +o pipefail`, and that should fix it again.

Does this make sense?
> ah, sorry, didn't see in the code you were already using the `##vso`. I guess that's some azure magic then, but will be applied to all the other commands too, so it's ok.
> 
> I'd prefer to remove the `awk` and leave the original call when not using azure. Will be misleading if the errors are different when people call `isort` directly than with our script I think.
> 
> And the thing that concerns me is the `set -o pipefail` inside the if at the beginning. If eventually we change that code, your code will fail. And that doesn't seem right, since both things are unrelated, and shouldn't be using side-effects of each other. I'd start by adding `set +o pipefail` after the grep in line 39. That should already be there, will avoid the side effects, and will make your code fail. After that, you can wrap your code with the same `set -o pipefail` and `set +o pipefail`, and that should fix it again.
> 
> Does this make sense?

I think so, I'll take a look at this tonight! Thanks for the info. : ) (will ping if I can't get it!)
@leeyspaul can you merge master again? Old builds are no longer available
@leeyspaul is this still active?
@WillAyd hey yeah, sorry. Had gotten caught up with work. I will work on this this weekend.
Great! No worries on timing - not in any rush. Just wanted to make sure this was something that you were still looking at, so thanks for confirming
@leeyspaul can you merge master and update
@leeyspaul can you rebase
If we dont hear from @leeyspaul I can take this on
@alimcmaster1 feel free to take over. I don't think I'll be able to complete this any time soon. Thanks!
Restarted this over here: https://github.com/pandas-dev/pandas/pull/29654

Closing this for now
I'm moving the Checks build to actions, and I think it only supports the `##[error]` formatting, not the one with file, line number...
Ahh so there probably isn’t much value in this at all? I can close my PR if so 
There is surely value on letting the CI know what's an error. Just the format will probably be different.