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

Remove isort, enable ruff's isort rules and apply them #5055

Merged
merged 7 commits into from Jan 8, 2024

Conversation

glennmatthews
Copy link
Contributor

Closes: #n/a

What's Changed

  • Remove isort dev dependency (was never automatically used or enforced) - note that it actually stays in poetry.lock because it's a transitive dependency of pylint, but we can't do anything about that for now...
  • Enable Ruff I rule set and configure it in pyproject.toml to most closely approximate our current conventions (I'm absolutely open to suggested changes here, see https://docs.astral.sh/ruff/settings/#isort for options)
  • In the example_plugin, configure nautobot as its own group of imports to get placed before imports from within the example plugin, as this seems to match our current convention most closely.
  • Run invoke ruff --fix and let it rearrange imports to its heart's content.
    • Due to some known circular dependencies between nautobot.core.models and nautobot.extras.models, I had to add # isort:skip to prevent a very small number of changes in nautobot.circuits.models and nautobot.dcim.models.cables that would have triggered circular import errors if moved.

TODO

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • n/a Attached Screenshots, Payload Example
  • n/a Unit, Integration Tests
  • n/a Documentation Updates (when adding/changing features)
  • Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@glennmatthews glennmatthews added type: housekeeping Changes to the application which do not directly impact the end user emergent Unplanned work that is brought into a sprint after it's started. labels Jan 4, 2024
@glennmatthews glennmatthews self-assigned this Jan 4, 2024
Copy link
Contributor

@HanlinMiao HanlinMiao left a comment

Choose a reason for hiding this comment

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

In YOLOs we trust. The dependency changes look good, but I only looked through a quarter of the change incurred due to the applications of isort rules.

Comment on lines +35 to +48
ignore = [
"E501", # pycodestyle: line-too-long
"PIE808", # unnecessary-range-start
"RUF012", # mutable-class-default - see https://github.com/astral-sh/ruff/issues/5243
]

[tool.ruff.lint.isort]
combine-as-imports = true
force-sort-within-sections = true
order-by-type = false
section-order = ["future", "standard-library", "third-party", "nautobot", "first-party", "local-folder"]

[tool.ruff.lint.isort.sections]
"nautobot" = ["nautobot"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea why GitHub is considering this invalid - as far as I can tell it's correct TOML?

Copy link
Contributor

Choose a reason for hiding this comment

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

Github does not like a comment on the last line of the list within toml... seems like a github bug, will see if I can open an issue.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Opened github support ticket 2514042 on it

@gsnider2195
Copy link
Contributor

Can you update docs/development/core/style-guide.md ## Importing Python Packages section?

@glennmatthews
Copy link
Contributor Author

Can you update docs/development/core/style-guide.md ## Importing Python Packages section?

Oof, I'd forgotten that we had such detailed prescriptivism in the docs that isn't currently enforced by any tooling. At a quick glance, I don't see exactly what needs to be updated for this PR though - any specific pointers?

@gsnider2195
Copy link
Contributor

Can you update docs/development/core/style-guide.md ## Importing Python Packages section?

Oof, I'd forgotten that we had such detailed prescriptivism in the docs that isn't currently enforced by any tooling. At a quick glance, I don't see exactly what needs to be updated for this PR though - any specific pointers?

I was thinking just an annotation on how to use ruff to format your imports somewhere near the top.

@glennmatthews
Copy link
Contributor Author

Added a note to the docs and improved the invoke ruff task based on John's feedback.

@glennmatthews glennmatthews merged commit ce9276d into develop Jan 8, 2024
17 checks passed
@glennmatthews glennmatthews deleted the u/glennmatthews-ruff-isort branch January 8, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emergent Unplanned work that is brought into a sprint after it's started. type: housekeeping Changes to the application which do not directly impact the end user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants