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.

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

Reduced the list of ignored errors by flake8 #1598

Closed
wants to merge 1 commit into from

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Feb 1, 2021

Description:

  • Reduced the list of ignored errors by flake8
  • fixed errors

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

@sdesrozis @fco-dv any ideas why code formatting does not pass here, but passes locally without problem ?
I repeat the same steps with same versions etc : https://github.com/pytorch/ignite/pull/1598/checks?check_run_id=1807298100#step:4:1
and nothing exceptional shown...
Could you please try to repro from your side ?

@Devanshu24
Copy link
Contributor

Devanshu24 commented Feb 1, 2021

I also faced this in one of my PRs, I think (I am not sure) it could be one of these things

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

Thanks for pointers @Devanshu24 ! I think, my actual problem is not with git-auto-commit-action but with the fact that
code formatting step fixes unformatted code : https://github.com/pytorch/ignite/pull/1598/checks?check_run_id=1807298100#step:4:40
but I can not repro this locally with the same tools etc...
Could you please reproduce the issue locally from your side ? By repro, I mean to see that we need to reformat with some tool

Fixing examples/mnist/mnist_save_resume_engine.py
Fixing examples/mnist/mnist.py
Fixing examples/fast_neural_style/utils.py
Fixing examples/references/segmentation/pascal_voc2012/code/dataflow/vis.py
Fixing examples/references/segmentation/pascal_voc2012/code/dataflow/datasets.py

@Devanshu24
Copy link
Contributor

Just checked out this PR locally and ran all the steps given below

python -m pip install autopep8 "black==19.10b0" "isort==4.3.21"
isort -rc .
autopep8 --recursive --in-place --aggressive --aggressive .
black .

Output for each commands

$ isort -rc .
Skipped 11 files
$ autopep8 --recursive --in-place --aggressive --aggressive .

(no output for autopep8)

$ black .
All done! ✨ 🍰 ✨
238 files left unchanged.

Even I could not get the following lines after following all the steps the workflow config file (totally clueless why)

Fixing examples/mnist/mnist_save_resume_engine.py
Fixing examples/mnist/mnist.py
Fixing examples/fast_neural_style/utils.py
Fixing examples/references/segmentation/pascal_voc2012/code/dataflow/vis.py
Fixing examples/references/segmentation/pascal_voc2012/code/dataflow/datasets.py

Are you aware which of the tools actually produces this kind of output (isort or autopep8 or black) maybe that could help us find how is this happening

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

I think it is isort. If you change the order in one already correctly sorted files it will try to fix it and the message is like :

Fixing /ignite/examples/contrib/mnist/mnist_with_tensorboard_logger.py
Skipped 13 files

We also run code formatting check in unit-tests acitons, for example:
https://github.com/pytorch/ignite/runs/1807298131?check_suite_focus=true#step:8:1
and it passes ))

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

@ydcjeff do you have any idea why code-style action is irreproducible in some sort ?

@trsvchn
Copy link
Collaborator

trsvchn commented Feb 1, 2021

@vfdev-5 Maybe that's because of pre-commit hooks? And that's why it works locally

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

@trsvchn thanks for joining :) Actually I'm not using precommit :) and I just running it in a conda env...

Strange thing here is that it uses python 3.8.7 that I could not even install with conda. I tested on 3.8.5. I'm now suspecting every difference with my setup in the logs. This warning seems weird:
https://github.com/pytorch/ignite/pull/1598/checks?check_run_id=1808076330#step:4:52

UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.

@Devanshu24
Copy link
Contributor

Devanshu24 commented Feb 1, 2021

One possible way to debug is maybe to fix the problem with git-auto-commit-action and see what are the new changes committed. Maybe it will help us to understand what exactly are the changes that isort is doing

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

@Devanshu24 yes, makes sense. In this case, I also wonder if other steps where we only check the formatting will pass ...

@ydcjeff
Copy link
Contributor

ydcjeff commented Feb 1, 2021

Just ran a quick check https://github.com/ydcjeff/ignite/runs/1808526597?check_suite_focus=true
No error locally and failing in CI

@trsvchn
Copy link
Collaborator

trsvchn commented Feb 1, 2021

Well I've found issue where isort and this setuptools warning are mentioned together

microsoft/vscode-python#12949

@trsvchn
Copy link
Collaborator

trsvchn commented Feb 1, 2021

ok, I've tried to reproduce the issue, I added these:

trsvchn@712baa1

And no errors:

https://github.com/trsvchn/ignite/actions/runs/528601462

Am I missing smth?

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

@trsvchn so, if understand correctly, you removed ignored errors from setup.cfg as I did in my PR and let run code style formatting to format the code, right ?

EDIT: Let me try to do the same ! Thanks for the suggest !

@trsvchn
Copy link
Collaborator

trsvchn commented Feb 1, 2021

@vfdev-5 yes, I removed examples/**, some errors from ignore list and add python -m pip install --upgrade pip wheel to code-style.yml

@vfdev-5 vfdev-5 mentioned this pull request Feb 1, 2021
3 tasks
@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 1, 2021

I close this PR in favor of #1602

Thanks a lot everyone who participated in the discussion, appreciate a lot your help !

@vfdev-5 vfdev-5 closed this Feb 1, 2021
@vfdev-5 vfdev-5 deleted the flake8-fix-ignored-errors branch February 1, 2021 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants