Skip to content

Conversation

@anmolsjoshi
Copy link
Contributor

@anmolsjoshi anmolsjoshi commented Feb 18, 2020

Fixes #

Description:
Per discussion on slack, added black formatting to codebase.

Helpful links:

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)

@anmolsjoshi anmolsjoshi requested a review from vfdev-5 February 18, 2020 23:40
@anmolsjoshi
Copy link
Contributor Author

cc @justusschock @ykumards

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 18, 2020

Thanks @anmolsjoshi ! Huge PR with 164 updated files, if CI passes it is OK for me.
I let Justus and Yogesh say their word too...

@justusschock
Copy link
Contributor

I just got one small question: can't we use black also as a linter? This way it would be more concise than to format with black and to lint with flake and additional exceptions.

@ykumards
Copy link
Contributor

Thanks @anmolsjoshi ! Huge PR with 164 updated files, if CI passes it is OK for me.
I let Justus and Yogesh say their word too...

love this!

@anmolsjoshi
Copy link
Contributor Author

anmolsjoshi commented Feb 19, 2020

@justusschock black is primarily a code formatter. We can invoke it with a --check flag to check if all the files in the code base comply with, but it doesn't suggest what should be changed like flake8. Please see below.

black . --check
would reformat /home/joshi/ignite/tests/ignite/metrics/test_metric.py
would reformat /home/joshi/ignite/tests/ignite/metrics/test_accuracy.py
would reformat /home/joshi/ignite/tests/ignite/metrics/test_precision.py
would reformat /home/joshi/ignite/tests/ignite/metrics/test_recall.py
Oh no! 💥 💔 💥
165 files would be reformatted, 19 files would be left unchanged.

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks a lot @anmolsjoshi !

@vfdev-5 vfdev-5 merged commit 6040d45 into pytorch:master Feb 19, 2020
@anmolsjoshi anmolsjoshi deleted the feature/precommit branch February 19, 2020 14:35
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.

4 participants