Superflore Tooling Modernization by robwoolley · Pull Request #329 · ros-infrastructure/superflore

robwoolley · 2026-08-20T11:24:49Z

I used Claude to assist with modernizing the tooling for Superflore. The details are listed below. The commits have been manually reviewed for accuracy.

This pull request is being shared to gather feedback from downstream users of superflore and to perform additional testing on GitHub.

Goal: modernize packaging, linting, and testing tooling with no change to
runtime behavior. Milestones are ordered must-do → recommended → nice-to-have
(see the accompanying recommendations for the full rationale and tradeoffs).
Flag-only items (stdeb.cfg, setup_superflore.sh) are intentionally
excluded — they need an owner decision before any milestone touches them.

Each milestone should land as its own PR so CI validates it independently.

Must-do

M1 — Migrate packaging to `pyproject.toml`

Depends on: nothing (do first).

Tasks:

Add pyproject.toml with [project] metadata (name, description, authors,
license, classifiers, python_requires, console_scripts entry points)
ported from setup.py.
Adopt setuptools-scm for versioning, replacing the custom
append_local_version_label() git-describe logic. Configure via
[tool.setuptools_scm]; confirm the resulting version string is still
PEP 440-compliant and acceptable to anything that parses it downstream
(release scripts, changelog tooling).
Split dependencies:
- Runtime deps (xmltodict, termcolor, gitpython, requests,
  docker, pyyaml, pygithub, rosinstall_generator, rosdistro,
  rosdep, catkin_pkg, rospkg, setuptools) → [project.dependencies].
- Dev/lint/test tools (pynose, flake8, flake8-import-order, pep8)
  → a dev/test group under [project.optional-dependencies] or
  [dependency-groups]. These get replaced in M3/M2 but should move as-is
  here so this milestone stays packaging-only.
Remove setup.py once pyproject.toml fully covers it, unless
setuptools-scm or the build backend needs a minimal shim — confirm with a
clean pip install . and python -m build.
Decide whether requirements.txt is retired (replaced by
pip install -e ".[dev]") or kept as a pinned/lock-style install path.

Acceptance criteria:

pip install . and pip install -e ".[dev]" both work from a clean venv.
python -m build produces a valid sdist + wheel with correct version
string (test against an actual git tag and against a dirty/untagged tree).
All three console_scripts entry points still resolve and run --help.
CI installs successfully via the new metadata.

M2 — Migrate test suite from nose to pytest

Depends on: M1 (so the dev/test dependency group exists to add
pytest to and remove pynose from).

Tasks:

Add pytest to the test dependency group; remove pynose.
Replace the CI invocation python -m 'nose' --exclude ... --ignore-files ...
(ci.yaml) with pytest.
Convert the exclude/ignore list currently embedded in the CI command line
(test_pull, test_run, test_logger_output, test_failed_to_create,
test_generate_installers, test_ebuild.py, test_docker.py) into
in-file pytest.mark.skip(reason=...) / pytest.mark.xfail(reason=...)
markers, so each skip carries its reason next to the test instead of in
CI YAML.
Confirm existing test discovery still works with pytest's conventions
(the current tests/__init__.py-based package layout is compatible;
no restructuring expected).
Do not touch test_flake8/test_pep8 in
tests/test_code_format.py yet — that's M3.

Acceptance criteria:

pytest run locally and in CI produces the same pass/fail set as the old
nose invocation (module the previously-excluded tests, which now show as
explicit skips/xfails instead of being invisible).
No test is silently dropped — every prior --exclude/--ignore-files
entry has a corresponding marker with a stated reason.

Add ruff to the dev dependency group; remove flake8,
flake8-import-order, and pep8 (the deprecated pycodestyle package name).
Add [tool.ruff] config to pyproject.toml covering lint rules and import
sorting, replacing the --import-order-style=google flag that's currently
passed at the CLI in two places
(ci.yaml and
tests/test_code_format.py).
Run ruff check and ruff format once across the codebase and commit the
resulting reformat as its own commit, separate from any config changes, so
the diff is reviewable.
Delete test_flake8 and test_pep8 from
tests/test_code_format.py — linting as a
pytest test duplicates the CI lint step and drags in the dead
sys.version_info < (3, 4) Python-2-compat branches. If the file ends up
empty, delete it.
Spot-check the import-order diff against google-style expectations —
Ruff's isort-compatible ordering isn't byte-identical to
flake8-import-order's google style, so this is the one step in the whole
spec with a real (if small) risk of unwanted churn.

Acceptance criteria:

ruff check superflore tests and ruff format --check superflore tests
both pass clean in CI.
No references to flake8, flake8-import-order, or pep8 remain in
dependency metadata, CI, or test files.
Import-order diff reviewed and confirmed non-disruptive.

M4 — Add pre-commit configuration

Depends on: M3 (Ruff config must exist to wire in).

Tasks:

Add .pre-commit-config.yaml running ruff check --fix and ruff format
(using the ruff-pre-commit hook repo) against the same config as CI.
Document the one-time pre-commit install setup step in
README.md (contributor-facing section).

Acceptance criteria:

pre-commit run --all-files passes clean.
Hook versions pinned (not floating main/master refs).

M5 — Update CI workflow

Depends on: M2, M3 (pytest and Ruff must already be the tools in use).

Tasks:

Replace the nose and flake8 invocations in
ci.yaml with pytest and
ruff check / ruff format --check.
Add pip caching (actions/setup-python's cache: pip, keyed off
pyproject.toml) so the 10-way matrix (2 OS × 5 Python versions) isn't
reinstalling everything from scratch on every run.
No changes to the ROS/rosdep apt bootstrap steps — out of scope, unrelated
to Python tooling.

Acceptance criteria:

Full CI matrix (ubuntu-22.04/24.04 × Python 3.9–3.13) passes green.
Measurable reduction in per-job install time versus the pre-M5 baseline.

Nice-to-have

M6 — Introduce type checking (incremental)

Depends on: M1–M5 complete (this is additive polish, not a prerequisite
for anything above).

Tasks:

Add mypy (or pyright) to the dev dependency group.
Start in a non-blocking mode: run in CI as a report-only step (does not
fail the build) against the existing ~5 files that already touch
typing, with the rest of the codebase excluded via config.
Expand coverage opportunistically as files are touched for other reasons,
rather than as a dedicated sweep — the codebase has almost no existing
type coverage, so a full pass isn't worth front-loading.

Acceptance criteria:

Type checker runs in CI without blocking merges.
Config documents which modules are checked vs. excluded, so the boundary
is explicit rather than implicit.

robwoolley · 2026-08-20T11:39:32Z

@wentasah Please let me know what you think of these changes? It shouldn't affect the Nix generator, but it may affect how you install it. My hope is to release a new package version to PyPi so we can have formally tested releases.

robwoolley · 2026-08-20T11:42:49Z

@cottsay I would love any feedback you have on the tooling and CI tools selected. Using ruff and mypy seems like the way to go, but I am open to any alternatives or additions you suggest.

robwoolley · 2026-08-20T11:45:08Z

@tfoote Is there anything else that I should add or do to work towards publishing a new version (v0.4.0) to PyPI? I have published packages both manually as well as with GitHub Actions using the "Trusted Publishing" mechanism. What would you recommend?

wentasah · 2026-08-20T14:17:15Z

@robwoolley It works for me. I use two small patches, which I haven't sent you yet. It was necessary to resolve conflicts caused by reformatting the code, but it was easy. Modernization required minor changes in packaging on my side. I verified that with these changes, it generates exactly the same output as before.

The only thing that I noticed are the following warnings:

pypa build flags: --no-isolation --outdir dist/ --wheel
* Getting build dependencies for wheel...
/nix/store/fkfl68z8s63y73y66398j7jrzvqdsddp-python3.14-setuptools-83.0.0/lib/python3.14/site-packages/setuptools/config/_apply_pyprojecttoml.py:82: SetuptoolsDeprecationWarning: `project.license` as a TOML table is deprecated
!!

        ********************************************************************************
        Please use a simple string containing a SPDX expression for `project.license`. You can also use `project.license-files`. (Both options available on setuptools>=77.0.0).

        By 2027-Feb-18, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.

        See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
        ********************************************************************************

!!
  corresp(dist, value, root_dir)
/nix/store/fkfl68z8s63y73y66398j7jrzvqdsddp-python3.14-setuptools-83.0.0/lib/python3.14/site-packages/setuptools/config/_apply_pyprojecttoml.py:61: SetuptoolsDeprecationWarning: License classifiers are deprecated.
!!

        ********************************************************************************
        Please consider removing the following classifiers in favor of a SPDX license expression:

        License :: OSI Approved :: Apache Software License
        License :: OSI Approved :: MIT License

        See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
        ********************************************************************************

!!
  dist._finalize_license_expression()
/nix/store/fkfl68z8s63y73y66398j7jrzvqdsddp-python3.14-setuptools-83.0.0/lib/python3.14/site-packages/setuptools/dist.py:765: SetuptoolsDeprecationWarning: License classifiers are deprecated.
!!

        ********************************************************************************
        Please consider removing the following classifiers in favor of a SPDX license expression:

        License :: OSI Approved :: Apache Software License
        License :: OSI Approved :: MIT License

        See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
        ********************************************************************************

!!
  self._finalize_license_expression()

Maybe, these can be addressed somehow in the update.

robwoolley · 2026-08-20T17:13:54Z

@robwoolley It works for me. I use two small patches, which I haven't sent you yet. It was necessary to resolve conflicts caused by reformatting the code, but it was easy. Modernization required minor changes in packaging on my side. I verified that with these changes, it generates exactly the same output as before.

The only thing that I noticed are the following warnings:

pypa build flags: --no-isolation --outdir dist/ --wheel
* Getting build dependencies for wheel...
/nix/store/fkfl68z8s63y73y66398j7jrzvqdsddp-python3.14-setuptools-83.0.0/lib/python3.14/site-packages/setuptools/config/_apply_pyprojecttoml.py:82: SetuptoolsDeprecationWarning: `project.license` as a TOML table is deprecated
!!

        ********************************************************************************
        Please use a simple string containing a SPDX expression for `project.license`. You can also use `project.license-files`. (Both options available on setuptools>=77.0.0).

        By 2027-Feb-18, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.

        See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
        ********************************************************************************

!!
  corresp(dist, value, root_dir)
/nix/store/fkfl68z8s63y73y66398j7jrzvqdsddp-python3.14-setuptools-83.0.0/lib/python3.14/site-packages/setuptools/config/_apply_pyprojecttoml.py:61: SetuptoolsDeprecationWarning: License classifiers are deprecated.
!!

        ********************************************************************************
        Please consider removing the following classifiers in favor of a SPDX license expression:

        License :: OSI Approved :: Apache Software License
        License :: OSI Approved :: MIT License

        See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
        ********************************************************************************

!!
  dist._finalize_license_expression()
/nix/store/fkfl68z8s63y73y66398j7jrzvqdsddp-python3.14-setuptools-83.0.0/lib/python3.14/site-packages/setuptools/dist.py:765: SetuptoolsDeprecationWarning: License classifiers are deprecated.
!!

        ********************************************************************************
        Please consider removing the following classifiers in favor of a SPDX license expression:

        License :: OSI Approved :: Apache Software License
        License :: OSI Approved :: MIT License

        See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
        ********************************************************************************

!!
  self._finalize_license_expression()

Maybe, these can be addressed somehow in the update.

Thanks for the feedback! I have pushed another commit which should address the problem you saw.

tfoote · 2026-08-21T04:28:23Z

We've historically done it manually, but setting up a trusted automated publishing would be even better. I'd suggest talking to the infrastructure pmc about it. Cc @cottsay

Includes a duplicate entry for wrynose and formatting changes. Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

Replace setup.py and requirements.txt with PEP 621 metadata in pyproject.toml. Versioning moves from a hand-rolled git-describe function to setuptools-scm, and runtime deps are separated from the existing test/dev toolchain via optional-dependencies. CI and the README install instructions are updated to match. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

Swap pynose for pytest in CI and the test dependency group; the existing unittest.TestCase-based tests needed no code changes to run under pytest. The nose --exclude/--ignore-files flags that were silently skipping tests from the CI command line are replaced with in-file @unittest.skip markers carrying the original reasons, so skipped coverage is visible in test output instead of hidden in a CI YAML line. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

Replace flake8, flake8-import-order, and pep8 with Ruff for linting, import sorting, and formatting; add its config to pyproject.toml and swap the CI/dev dependencies and workflow steps accordingly. The line-length limit moves from flake8's unconfigured default (79, only ever enforced against superflore/, never tests/) to Ruff's 88 applied to both. Delete tests/test_code_format.py: running the linter a second time as a pytest test duplicated the new CI lint step and carried dead Python-2-compat branches. Fix issues Ruff surfaced when it covered tests/ for the first time: tests/test_nix.py had two methods both named test_escape_quote, so the second silently shadowed the first under both nose and pytest; deduplicating the names exposed that the shadowed test's expected value was stale, now corrected to match NixLicense's actual (space -to-dash) escaping behavior. A handful of unused local variables in assertRaises blocks are also removed. Ruff's import sorting is isort-compatible but not identical to flake8-import-order's "google" style, so this also reflows most files' import blocks; `ruff format` was run once across superflore/ and tests/ for consistent style, configured to keep single-quote strings to minimize diff noise against the existing convention. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

Wire up .pre-commit-config.yaml running the same ruff-check/ruff-format hooks as CI, add pre-commit to the dev dependency group, and document the one-time `pre-commit install` setup step in the README. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

Enable actions/setup-python's pip cache, keyed on pyproject.toml, so the 10-way OS/Python matrix stops reinstalling every dependency from scratch on each run. The pytest/ruff invocations were already updated to their final form in the earlier commits so CI stayed green at every step. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

Add mypy to the dev dependency group and a report-only CI step (continue-on-error, so it never blocks a merge). Coverage is scoped via [tool.mypy].files to the handful of modules that already carry type annotations, with follow_imports=silent so errors from unlisted modules pulled in transitively don't leak into the report. Expand the files list opportunistically as modules gain annotations, rather than as a dedicated typing sweep -- the codebase has almost no existing type coverage. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

actions/checkout@v4 defaults to a shallow, tag-less clone. With no tags reachable, setuptools-scm falls back to its default base version (0.1.devN+g<hash>), which fails test_get_superflore_version's sanity check that the version isn't lower than expected. Reproduced locally: a --depth 1 clone reads back '0.1.dev1+g<hash>' and fails the test; a full clone correctly resolves to '0.3.4.devNN+g<hash>' and passes. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

Replace the "License :: OSI Approved" classifiers with a PEP 639 SPDX license expression. The project is Apache-2.0 overall, but two files (nix_expression.py, yocto_recipe.py) carry their own standalone MIT headers, so the correct expression is "Apache-2.0 AND MIT" (not "OR" -- it's not a licensee's choice, different files are under different licenses). Bumps the setuptools build requirement to >=77, which is what added SPDX license-expression support. Assisted-by: Claude:claude-opus-5 AI-Generated: Uses Claude (claude-opus-5) Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

robwoolley requested review from cottsay and tfoote August 20, 2026 11:37

robwoolley requested review from cottsay and removed request for cottsay August 27, 2026 11:39

robwoolley force-pushed the modernize-tooling branch 2 times, most recently from 3670fee to 61ea3c8 Compare September 1, 2026 16:03

robwoolley added 9 commits September 1, 2026 09:34

bitbake: Fix linting errors found by ruff

8742220

Includes a duplicate entry for wrynose and formatting changes. Signed-off-by: Rob Woolley <rob.woolley@windriver.com>

robwoolley force-pushed the modernize-tooling branch 3 times, most recently from 3978138 to 87f47ed Compare September 1, 2026 16:43

robwoolley requested review from claraberendsen and nuclearsandwich and removed request for cottsay, nuclearsandwich and tfoote September 1, 2026 16:57

Navigation Menu

Uh oh!

Superflore Tooling Modernization - #329

Superflore Tooling Modernization#329
robwoolley wants to merge 9 commits into
ros-infrastructure:masterfrom
robwoolley:modernize-tooling

robwoolley commented Aug 20, 2026

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

wentasah commented Aug 20, 2026

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

tfoote commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

robwoolley commented Aug 20, 2026

Must-do

M1 — Migrate packaging to pyproject.toml

M2 — Migrate test suite from nose to pytest

Recommended

M3 — Consolidate linting/formatting onto Ruff

M4 — Add pre-commit configuration

M5 — Update CI workflow

Nice-to-have

M6 — Introduce type checking (incremental)

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

wentasah commented Aug 20, 2026

Uh oh!

robwoolley commented Aug 20, 2026

Uh oh!

tfoote commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

M1 — Migrate packaging to `pyproject.toml`