Skip to content

Conversation

@bmos
Copy link
Contributor

@bmos bmos commented Feb 1, 2025

As per last year's discussion in #1132, here is a discrete PR implementing import sorting in the Github Actions check and pre-commit config.

See PEP #8 > Imports
https://peps.python.org/pep-0008/#imports

@bmos bmos marked this pull request as ready for review February 1, 2025 20:09
Copy link
Collaborator

@austinweisgrau austinweisgrau left a comment

Choose a reason for hiding this comment

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

I think we should use the ruff import sorter rather than bring isort in. Since we're already using ruff and ruff comes with an isort-like import sorter.

Ruff documentation on its import sorter: https://docs.astral.sh/ruff/faq/#how-does-ruffs-import-sorting-compare-to-isort

Ruff documentation on how to use its import sorter: https://docs.astral.sh/ruff/formatter/#sorting-imports

I don't see any conflicts between ruff and isort in the repo as-is, and can't find any description of this, but I know isort and black and have conflicting opinions about how to format imports, and I imagine that could be true for isort and ruff as well.

I think the main value here is reducing additional dependencies, which is a chronic problem with parsons.

@bmos
Copy link
Contributor Author

bmos commented Feb 3, 2025

How interesting, I had no idea that Ruff offered that.
I completely agree and I will rework this pull request this afternoon.

@bmos bmos force-pushed the isort branch 2 times, most recently from 67edde9 to 1fa0df7 Compare February 3, 2025 17:47
@bmos bmos changed the title Add isort import sorting Add import sorting Feb 3, 2025
@bmos bmos requested a review from austinweisgrau February 3, 2025 19:01
Copy link
Collaborator

@austinweisgrau austinweisgrau left a comment

Choose a reason for hiding this comment

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

How elegant

@bmos bmos force-pushed the isort branch 2 times, most recently from 236a4aa to 2f5c237 Compare February 4, 2025 18:09
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

looks great!

@shaunagm shaunagm merged commit de29454 into move-coop:main Feb 6, 2025
57 checks passed
@bmos bmos deleted the isort branch February 6, 2025 20:42
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.

3 participants