Skip to content

Update tooling to match other CFPB packages#3

Merged
niqjohnson merged 5 commits intomainfrom
ruff
Apr 10, 2026
Merged

Update tooling to match other CFPB packages#3
niqjohnson merged 5 commits intomainfrom
ruff

Conversation

@niqjohnson
Copy link
Copy Markdown
Member

@niqjohnson niqjohnson commented Apr 1, 2026

Updates our tooling to match other CFPB-maintained packages. See cfpb/wagtail-flags#77 for a similar update.

The actual impetus for this is that when running tox -e lint, black and isort wanted to make conflicting changes to a handful of files. Instead of fighting with that, I figured it was a good time to bring this package in line with the tooling in our other packages before updating it for Wagtail 7.

Additions

  • Pre-commit config
  • bandit for static security analysis
  • setuptools_scm for versioning so we won't have to manually update a version number in pyproject.toml

Removals

  • black and isort for linting

Changes

  • Use ruff exclusively for formatting, linting, and import-sorting. ruff can replace black and isort and is faster. We've replaced black and isort with ruff in most of our other packages and projects.
  • A few linting fixes identified by ruff
  • Bump actions dependencies to latest (see Bump all actions dependencies to latest consumerfinance.gov#9041)

Testing

  1. All tests, including linting, should pass with tox
  2. PR checks should pass

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the development playbook
  • Passes all existing automated tests
  • Reviewers requested with the Reviewers tool ➡️

Updates our tooling to match other CFPB-maintained packages:

- Use Ruff exclusively for formatting, linting, and import-sorting.
  Ruff can replace Black and isort, and is faster.
- Add Bandit for static security analysis.
- Add setuptools_scm for versioning.
  We won't have to manually update a version number in pyproject.toml.
- Add a pre-commit config.
- Run `ruff check --fix` for linting fixes.

See cfpb/wagtail-flags#77 for a similar update
Comment on lines +91 to +92
for row_index, row in enumerate(value.rows): # noqa: B007
for column_index, child in enumerate(row): # noqa: B007
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ruff suggested changing to _row_index and _column_index since they weren't used inside the loop, but I wasn't sure if that change would have an effect, so I ignored the lines. I can change them if that seems like an OK change.

Copy link
Copy Markdown
Member

@chosak chosak Apr 8, 2026

Choose a reason for hiding this comment

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

I suspect this could just be

for row in value.rows:
    for child in row:

The purpose of Python's built-in enumerate is to generate numeric indexes as you iterate over an iterable, useful for various purposes including debugging. Since we're not using those indexes (row_index and column_index), there's no reason to use enumerate at all. Maybe this was here because of some long-ago debugging? @willbarton do you remember?

@niqjohnson maybe try removing the enumerate and index variables entirely and see if tests still pass.

Copy link
Copy Markdown
Member

@willbarton willbarton Apr 8, 2026

Choose a reason for hiding this comment

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

I suspect it's from an earlier iteration of this code where the index served as part of the "path" to where a block value exists within the table. But I agree, it's not being used, so we can jettison enumerate entirely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, @chosak and @willbarton. All the tests still pass without the enumeration, which I removed in db89bf7.

Comment thread .pre-commit-config.yaml Outdated
Comment thread .pre-commit-config.yaml Outdated
Comment thread .pre-commit-config.yaml Outdated
Comment thread .pre-commit-config.yaml Outdated
@niqjohnson niqjohnson merged commit 8cf5b19 into main Apr 10, 2026
16 checks passed
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.

4 participants