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

Format all python code with black and isort - take 2 #4427

Merged
merged 7 commits into from
Aug 10, 2022

Conversation

justinchuby
Copy link
Contributor

Description

  • Format all python code with black and isort
  • Add format checks in CI
  • Skip additional generated files in isort

Motivation and Context

Standardize formatting

Fixes #4278

@justinchuby justinchuby changed the title Format all python code with black and isort Format all python code with black and isort - take 2 Aug 10, 2022
@justinchuby justinchuby marked this pull request as ready for review August 10, 2022 15:07
@justinchuby justinchuby requested a review from a team as a code owner August 10, 2022 15:07
@justinchuby
Copy link
Contributor Author

@jcwchen I created this pr to set up the config and CI for the formatters. Please let me know when the approved changes are merged and I can run the actual formatting process

Signed-off-by: Justin Chu <[email protected]>

update isort

Signed-off-by: Justin Chu <[email protected]>

update config

Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>

reset ci

Signed-off-by: Justin Chu <[email protected]>

fix linux ci

Signed-off-by: Justin Chu <[email protected]>
@justinchuby justinchuby force-pushed the justinchu/format-all-3 branch from cdab63a to 9ce9cfe Compare August 10, 2022 16:03
@jcwchen jcwchen added the run release CIs Use this label to trigger release tests in CI label Aug 10, 2022
@justinchuby justinchuby requested a review from a team as a code owner August 10, 2022 16:10
Signed-off-by: Justin Chu <[email protected]>
@justinchuby justinchuby force-pushed the justinchu/format-all-3 branch from bc1c304 to d650a8d Compare August 10, 2022 16:23
Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

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

output_X.pb should not be updated, right? The are updated because different platforms might produce different result from NumPy. To keep this PR simple, I would suggest not updating them. Thanks!

onnx/__init__.py Show resolved Hide resolved
@justinchuby
Copy link
Contributor Author

Do we check for output_X.pb in the CI? I ran tools/update_doc.sh and got those. I will revert the changes

Signed-off-by: Justin Chu <[email protected]>
@justinchuby justinchuby force-pushed the justinchu/format-all-3 branch from d650a8d to d8d9145 Compare August 10, 2022 17:25
@justinchuby
Copy link
Contributor Author

Done

Copy link
Member

@jcwchen jcwchen 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 for the quick PR: #4428. It indeed resolves a lot of unused variable warnings from onnx package.

@justinchuby
Copy link
Contributor Author

@gramalingam ready for your approval

@gramalingam gramalingam enabled auto-merge (squash) August 10, 2022 21:15
@gramalingam gramalingam merged commit fddb2b6 into onnx:main Aug 10, 2022
broune pushed a commit to broune/onnx that referenced this pull request May 6, 2023
* Add black and isort into requirement

Signed-off-by: Justin Chu <[email protected]>

update isort

Signed-off-by: Justin Chu <[email protected]>

update config

Signed-off-by: Justin Chu <[email protected]>

* Add ci

Signed-off-by: Justin Chu <[email protected]>

reset ci

Signed-off-by: Justin Chu <[email protected]>

fix linux ci

Signed-off-by: Justin Chu <[email protected]>

* Skip formatting the init file

Signed-off-by: Justin Chu <[email protected]>

* Black isort

Signed-off-by: Justin Chu <[email protected]>

* Gen docs

Signed-off-by: Justin Chu <[email protected]>

Signed-off-by: Justin Chu <[email protected]>
Co-authored-by: Chun-Wei Chen <[email protected]>
Co-authored-by: G. Ramalingam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run release CIs Use this label to trigger release tests in CI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[engineering] Sort import with isort
3 participants