Skip to content

Experiment: add Isort to our toolchain#1872

Merged
ChrisLovering merged 7 commits into
mainfrom
experiments/isort
Oct 16, 2021
Merged

Experiment: add Isort to our toolchain#1872
ChrisLovering merged 7 commits into
mainfrom
experiments/isort

Conversation

@Akarys42
Copy link
Copy Markdown
Contributor

@Akarys42 Akarys42 commented Oct 14, 2021

What is Isort?

PyCQA/ISORT is a fantastic hyper configurable tool that will magically sort your imports so you don't have to care about them.

It also comes with a precommit hook. Quite an awesome tool in my opinion.

What is this PR?

This is an experiment PR, we may want to merge it, we may not want to. I set up Isort with a style that matches our current one as much as possible, and generated a quite minimal diff (f9e9dca). If we do pursue this, we may want to look at other style options (I am thinking in particular sorting by types: having the constants first, then classes and then function) that will generate larger diffs, but at least aren't a breaking code change.

This PR includes:

  • an Isort installation with a style matching our current one
  • a precommit hook to run isort just before linting so you don't have to care about it
  • a new poetry task to run isort
  • a flake8 plugin checking the commit has indeed been sorted (flake8-isort)
  • the removal of the old flake8 plugin that checked the import order (flake8-import-order)
  • a fully sorted codebase
  • some bash madness to only check the license of our prod deps

@Akarys42 Akarys42 added t: feature New feature or request s: planning Discussing details p: 3 - low Low Priority a: tools Development related to our toolchain (Docker, pipenv, flake8) labels Oct 14, 2021
Copy link
Copy Markdown
Contributor

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

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

@python-discord-policy-bot python-discord-policy-bot Bot requested a review from a team October 15, 2021 08:47
Copy link
Copy Markdown
Contributor

@jchristgit jchristgit left a comment

Choose a reason for hiding this comment

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

I like this.

"some bash madness to only check the license of our prod deps"

why's that though? Does it break on isort?

Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

I concur

Comment thread pyproject.toml
Comment on lines +76 to +82
[tool.isort]
multi_line_output = 6
order_by_type = false
case_sensitive = true
combine_as_imports = true
line_length = 120
atomic = true
Copy link
Copy Markdown
Contributor

@Numerlor Numerlor Oct 16, 2021

Choose a reason for hiding this comment

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

Suggested change
[tool.isort]
multi_line_output = 6
order_by_type = false
case_sensitive = true
combine_as_imports = true
line_length = 120
atomic = true
[tool.isort]
multi_line_output = 6
order_by_type = false
case_sensitive = true
combine_as_imports = true
line_length = 120
atomic = true
known_first_party = ["bot"]

We should set the bot package as known first party so isort doesn't treat it as a dep

@ChrisLovering ChrisLovering merged commit 7ac8a9f into main Oct 16, 2021
@ChrisLovering ChrisLovering deleted the experiments/isort branch October 16, 2021 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tools Development related to our toolchain (Docker, pipenv, flake8) hacktoberfest-accepted p: 3 - low Low Priority s: planning Discussing details t: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants