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: yapf #5494

Merged
merged 32 commits into from
Mar 31, 2022
Merged

precommit: yapf #5494

merged 32 commits into from
Mar 31, 2022

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Nov 3, 2021

use YAPF as default codebase formatting
Resolves #4983
Alternative to, closes #5490

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

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Code improvements and bug fixes to enhance YOLOv5's functionalities.

📊 Key Changes

  • 🧹 Cleaned up code by enabling yapf formatting in .pre-commit-config.yaml.
  • ✨ Improved detect.py function definitions for better readability.
  • 🎨 Reformatted export.py for readability and standardized export formats.
  • 💅 Applied consistent style improvements to the codebase, such as line breaks and indentation.
  • 🛠 Fixed minor issues within the code files like hubconf.py, models/common.py, and models/experimental.py.

🎯 Purpose & Impact

  • 👀 These changes make the code more readable and maintainable.
  • 🐛 Bug fixes prevent unexpected behavior and facilitate seamless development and deployment.
  • 🔨 Enabling yapf formatting in the pre-commit workflow ensures consistent code style automatically, preventing style-related issues in future contributions.

@Borda Borda marked this pull request as ready for review November 4, 2021 00:03
@Borda Borda marked this pull request as draft November 4, 2021 00:03
@Borda Borda force-pushed the pre-commit/yapf branch 2 times, most recently from 622cc9e to 630b3db Compare November 4, 2021 00:09
@Borda Borda mentioned this pull request Nov 4, 2021
@Borda Borda marked this pull request as ready for review November 4, 2021 08:35
setup.cfg Show resolved Hide resolved
@Borda
Copy link
Contributor Author

Borda commented Nov 5, 2021

well seems that the ISORT and YAPF configuration yield in inconsistent imports formatting 😕

@glenn-jocher
Copy link
Member

@Borda yes I see that also :(

@Borda Borda force-pushed the pre-commit/yapf branch 2 times, most recently from 48f6980 to f78ff51 Compare November 9, 2021 14:33
@Borda
Copy link
Contributor Author

Borda commented Nov 9, 2021

yes I see that also :(

ok, found and motion which works for both - 5 - Hanging Grid Grouped

from third_party import (
    lib1, lib2, lib3, lib4,
    lib5, ...
)

@glenn-jocher mind checking if this is fine with you? :]

@Borda
Copy link
Contributor Author

Borda commented Nov 19, 2021

@glenn-jocher @kalenmike can we merge this one? :]

@Borda
Copy link
Contributor Author

Borda commented Dec 7, 2021

@glenn-jocher I would rather go with multi_line_output=5 as it is more readable, the 0 is so strange...

@Borda
Copy link
Contributor Author

Borda commented Dec 7, 2021

@Borda
Copy link
Contributor Author

Borda commented Dec 21, 2021

@glenn-jocher resolved conflicts, so shall be ready to go... 🐰

@Borda
Copy link
Contributor Author

Borda commented Jan 2, 2022

@glenn-jocher any update on this format cleaning PR? 😕

@Borda
Copy link
Contributor Author

Borda commented Jan 12, 2022

@glenn-jocher pls ping 🐰 ^^

@Borda
Copy link
Contributor Author

Borda commented Jan 17, 2022

@glenn-jocher Happy New Year 🎉

@Borda
Copy link
Contributor Author

Borda commented Jan 31, 2022

@glenn-jocher all resolved, I may suggest getting it to merge and you can experiment with other formations in a follow-up PR? :)

@Borda
Copy link
Contributor Author

Borda commented Feb 5, 2022

@glenn-jocher all shall be fixed now 🐰

@Borda
Copy link
Contributor Author

Borda commented Feb 8, 2022

@glenn-jocher still an ongoing discussion? 🐰

@Borda
Copy link
Contributor Author

Borda commented Feb 11, 2022

@glenn-jocher never-ending resolving conflicts become quite annoying... 😕

# Conflicts:
#	utils/plots.py
@glenn-jocher
Copy link
Member

@Borda ok I think I have a good set of yapf arguments now. PyCharm reformat accepts the yapf reformat without conflicts.

@Borda
Copy link
Contributor Author

Borda commented Mar 28, 2022

ok I think I have a good set of yapf arguments now. PyCharm reformat accepts the yapf reformat without conflicts.

perfect!

@glenn-jocher glenn-jocher merged commit c3d5ac1 into ultralytics:master Mar 31, 2022
@glenn-jocher
Copy link
Member

@Borda PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐

@Borda Borda deleted the pre-commit/yapf branch March 31, 2022 16:16
BjarneKuehl pushed a commit to fhkiel-mlaip/yolov5 that referenced this pull request Aug 26, 2022
* precommit: yapf

* align isort

* fix

# Conflicts:
#	utils/plots.py

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

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

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

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

* Update setup.cfg

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

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

* Update setup.cfg

* Update setup.cfg

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

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

* Update wandb_utils.py

* Update augmentations.py

* Update setup.cfg

* Update yolo.py

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

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

* Update val.py

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

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

* simplify colorstr

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

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

* val run fix

* export.py last comma

* Update export.py

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

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

* Update hubconf.py

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

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

* PyTorch Hub tuple fix

* PyTorch Hub tuple fix2

* PyTorch Hub tuple fix3

* Update setup

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

use formatting Isort/YAPF/Black
2 participants