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

precommit: isort #5493

Merged
merged 6 commits into from
Nov 4, 2021
Merged

precommit: isort #5493

merged 6 commits into from
Nov 4, 2021

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Nov 3, 2021

use Isort as default import formatting
Partial alternative to #4983

This PR defines Isort as pre-commit hook and lets the bot fix it... rabbit

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

This PR brings codebase improvements mainly by enlisting third-party tools for code clean-up and organizing imports.

📊 Key Changes

  • Enabled isort tool in .pre-commit-config.yaml to automatically sort the imports.
  • Refactoring import statements across multiple files like detect.py, export.py, hubconf.py, and others to be more organized and readable based on the isort standard.
  • Set isort configuration in setup.cfg, specifying a line length of 120 characters and a single-line output for multiple imports.
  • Reshuffling import statements and minor updates to spaces and blank lines to adhere to cleaner code standards.

🎯 Purpose & Impact

  • The use of isort 🔄 ensures that the imports are consistently formatted, which makes the code easier to read and maintain.
  • With better-structured imports, developers can understand dependencies at a glance 🧐.
  • This can potentially reduce merge conflicts related to import lines for contributors in the future ✨.
  • Streamlined code patterns contribute to a more stable and standardized codebase 🛠️.

@Borda Borda marked this pull request as ready for review November 4, 2021 00:10
@glenn-jocher
Copy link
Member

glenn-jocher commented Nov 4, 2021

@Borda awesome, I didn't know about isort! I looked at the results. Could we configure isort to create the default behavior shown on their README? Current PR is turning all multi-line imports into one-per-line, but README shows from file import (p1,p2,p3 etc.) utilizing full line width.

Current behavior (this PR)

Screen Shot 2021-11-04 at 4 39 52 PM

Desired behavior (https://github.com/PyCQA/isort README)

Screen Shot 2021-11-04 at 4 38 13 PM

@@ -43,3 +43,11 @@ ignore =
F403
E302
F541


[isort]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@glenn-jocher here is the configuration :]
feel free to edit it how you wish and let the bot update the PR

Copy link
Member

Choose a reason for hiding this comment

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

@Borda awesome!! Thanks I'll experiment with it.

@glenn-jocher glenn-jocher merged commit 0155548 into ultralytics:master Nov 4, 2021
@glenn-jocher
Copy link
Member

@Borda PR is merged. I reverted most of the isort settings back to default values except for line length 120. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐

@Borda Borda deleted the pre-commit/isort branch November 4, 2021 20:47
BjarneKuehl pushed a commit to fhkiel-mlaip/yolov5 that referenced this pull request Aug 26, 2022
* precommit: isort

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update isort config

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update name

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Glenn Jocher <[email protected]>
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.

2 participants