Skip to content

Modernize packaging + CI (setup.cfg -> pyproject, uv-stub CI) and fix two latent threading bugs - #2

Merged
thorwhalen merged 2 commits into
masterfrom
claude/rollout-modernize
Aug 4, 2026
Merged

Modernize packaging + CI (setup.cfg -> pyproject, uv-stub CI) and fix two latent threading bugs#2
thorwhalen merged 2 commits into
masterfrom
claude/rollout-modernize

Conversation

@thorwhalen

Copy link
Copy Markdown
Member

Part of the fleet-wide wads repo-modernization pass. This repo was still on the
legacy generation (setup.cfg + setup.py + a hand-rolled ~90-line workflow using
actions/setup-python@v2); it now matches the current standard: a hatchling
pyproject.toml as the single source of truth plus a 5-line ci.yml stub that
calls the reusable workflow i2mint/wads/.github/workflows/uv-ci.yml@master.

⚠️ Merging this PR will publish a new release to PyPI. Pushing to master
runs the publish job, which bumps the version from the declared 0.1.3 and
uploads. 0.1.3 is deliberately the version currently on PyPI, so the automatic
bump lands on the correct next patch rather than re-publishing a version behind
the released one.

1. Packaging: setup.cfg -> pyproject.toml

Full inventory of what was in setup.cfg and where it now lives:

setup.cfg pyproject.toml
name = pchealthstream2py [project].name
version = 0.1.3 [project].version
description [project].description
long_description = file:README.md + long_description_content_type [project].readme = "README.md"
url [project.urls].Homepage
license = apache-2.0 [project].license = "Apache-2.0" (SPDX string form; LICENSE file kept, and it is a real Apache-2.0 text)
install_requires = psutil, stream2py [project].dependencies
packages = find: / include_package_data = True [tool.hatch.build.targets.wheel].packages
platforms = any Operating System :: OS Independent classifier
zip_safe, root_url, display_name, description_file no pyproject equivalent (setuptools-only / wads-internal); dropped intentionally

There were no entry points / console scripts, no package data files, no
extras, no MANIFEST.in and no requirements.txt — nothing else to carry over.
Verified against real imports with wads-deps: the only module-level
third-party imports in the package are psutil and stream2py, which is
exactly what is declared.

Added while here: requires-python = ">=3.10", keywords, classifiers, authors,
Repository / Documentation URLs, the ecosystem-standard [tool.ruff] block
(without it the repo falls through to ruff's moving default and goes red on
unrelated style drift), and the standard .editorconfig.

Manual fixes to the three known rough edges of wads-migrate setup-to-pyproject:
the deprecated [project.license] text = ... table -> SPDX string; the empty
[tool.wads.ci].project_name -> "pchealthstream2py" (it is the target of CI's
ruff-check and coverage steps); and testpaths — see next section.

testpaths — this one actually mattered

wads CI runs pytest --doctest-modules with no path argument, so collection is
driven entirely by testpaths. The migration tool writes ["tests"]
unconditionally, and this repo has no top-level tests/ dir — its tests live in
pchealthstream2py/tests/. Left as generated, CI would have collected nothing
and still reported green. Set to ["pchealthstream2py"], which also matches what
the old workflow ran (pytest --doctest-modules -v $PROJECT_NAME).

Verified in the branch CI run: collected 3 items, 3 passed.

2. CI: legacy workflow -> uv stub

wads-migrate ci-to-uv then wads-migrate ci-to-stub. The old workflow
referenced secrets.PYPI_USERNAME and secrets.PYPI_PASSWORD; the uv CI uses
token-only PyPI auth, so only PYPI_PASSWORD is passed through by the stub and
PYPI_USERNAME is no longer needed. No other secrets were referenced, so
[tool.wads.ci.env] stays empty.

3. Two latent threading bugs fixed (separate commit)

Both are reproducible on the released code and were surfaced while validating
the migration.

a. The stop flag shadowed threading.Thread._stop. StatusInfoReader
subclasses threading.Thread and declared _stop: threading.Event. But
Thread._stop is a method CPython calls internally from
_wait_for_tstate_lock(), which is reached from both join() and is_alive().
On the released version:

reader.join(timeout=2)
TypeError: 'Event' object is not callable

and is_alive() could never flip to False after the worker finished, because
the interpreter's own bookkeeping call raised. Renamed to _stop_event.

b. All readers shared one queue and one stop flag. _index, _data,
_stop and _bt were class attributes, so every StatusInfoReader instance
shared a single SyncQueue and a single Event: opening a second reader
cleared the first one's buffer, closing either stopped both, and both readers'
items interleaved in one queue. Moved to per-instance state in __init__.

Also corrected network_download_speed's return annotation: dict or None
evaluates to plain dict at class-creation time, so the "or None" was silently
discarded. Now Optional[dict], which is what the body actually returns.

Two focused regression tests were added alongside the fixes (the only new tests
in this PR — no test-coverage sweep was attempted here).

Deliberately left undone

  • StatusInfoReader still cannot be re-opened after close() — a
    threading.Thread can only be started once, while SourceReader's documented
    contract (its own class docstring) allows close-then-reopen. Fixing it means
    having a thread rather than being one, which changes
    isinstance(reader, threading.Thread), so it is out of scope for a packaging
    pass. Filed as StatusInfoReader cannot be re-opened after close() (SourceReader contract violation) #1 with a repro and a suggested fix.
  • No ruff format sweep. The code is still single-quoted; CI's publish job
    formats and pushes back on merge, as it does fleet-wide. Keeping it out here
    keeps this diff reviewable.
  • No docs, type annotations, py.typed or agent-skills work — separate
    dimensions of the rollout.

Verification

Result
Local gate, baseline (before any change) pchealthstream2py pass 4.1s — 1 suite: 1 pass, 0 fail
Local gate, final pchealthstream2py pass 4.4s — 1 suite: 1 pass, 0 fail
Local pytest pchealthstream2py --doctest-modules -q 3 passed
Local ruff check pchealthstream2py (0.16.0) All checks passed
Local uv build sdist + wheel build clean; wheel contains the package and LICENSE
Branch CI run https://github.com/i2mint/pchealthstream2py/actions/runs/30860120932success

Branch CI jobs: Read Configuration ✅, Validation (3.10) ✅, Validation (3.12) ✅,
Windows Tests ✅, Publish skipped (branch is not the default branch), Publish
GitHub Pages skipped.

Repo audit (python -m wads.repo_audit) before -> after: the HIGH no pyproject.toml, MEDIUM legacy packaging files present and MEDIUM CI is the legacy generation findings all clear, as does LOW missing .editorconfig.
ci_format goes legacy -> uv-stub, legacy_files goes to empty.

The only new finding is LOW GitHub topics (none) != slugified pyproject keywords — a direct consequence of this PR filling in the previously-empty
keywords. The repo's GitHub topics and homepage have been aligned to match
(repo settings, not part of this diff). The remaining LOW findings (no agent skills) are other dimensions of the rollout, untouched here.

Migrates this repo from the legacy setup.cfg/setup.py + hand-rolled
GitHub Actions workflow to the current wads standard: a hatchling
pyproject.toml as the single source of truth, and a 5-line ci.yml stub
that calls the reusable workflow i2mint/wads/.github/workflows/uv-ci.yml.

Everything declared in setup.cfg is carried across:

  name, version (0.1.3, matching the released PyPI version), description,
  url -> [project.urls].Homepage, long_description(+content_type) -> readme,
  license apache-2.0 -> SPDX `license = "Apache-2.0"`,
  install_requires (psutil, stream2py) -> [project].dependencies,
  packages = find: / include_package_data -> hatchling wheel target.

There were no entry points, console scripts, package data files, extras,
MANIFEST.in or requirements.txt to preserve. `zip_safe` and the
wads-specific `root_url` / `display_name` / `description_file` keys have
no pyproject equivalent and are dropped intentionally.

Also:

- `testpaths = ["pchealthstream2py"]` (not the generated default
  `["tests"]`). wads CI runs `pytest --doctest-modules` with no path
  argument, so collection is driven entirely by testpaths; the repo has
  no top-level tests/ dir, so `["tests"]` would have silently collected
  nothing while still reporting green. Verified locally: 1 passed.
- `[tool.wads.ci].project_name` set to the real package name (the
  migration tool emits an empty string, which is the target of CI's
  ruff-check and coverage steps).
- SPDX string licence form instead of the deprecated
  `[project.license] text = ...` table the tool emits.
- Added the ecosystem-standard [tool.ruff] block, keywords, classifiers,
  authors, requires-python and Repository/Documentation URLs.
- Added the standard .editorconfig.

PYPI_USERNAME is no longer referenced: the uv CI uses token-only PyPI
auth via PYPI_PASSWORD, which the stub passes through explicitly.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
Both were surfaced while validating the packaging migration and are
reproducible on the released code.

1. The stop flag was named `_stop`, which shadows `threading.Thread._stop`
   — a real method CPython calls internally from `_wait_for_tstate_lock()`,
   reached via `join()` and `is_alive()`. Consequences on the released
   version:

     reader.join(timeout=2)
     TypeError: 'Event' object is not callable

   and `is_alive()` could never transition to False once the worker had
   finished, because the interpreter's own bookkeeping call blew up.
   Renamed to `_stop_event`; `Thread._stop` is left alone.

2. `_index`, `_data`, `_stop` and `_bt` were *class* attributes, so every
   StatusInfoReader instance shared one SyncQueue and one stop Event.
   Opening a second reader cleared the first reader's buffer, closing
   either reader stopped both, and the two readers' items interleaved in
   a single queue. Moved to per-instance state in `__init__`.

Also corrected `network_download_speed`'s return annotation: `dict or None`
evaluates to plain `dict` at class-creation time, so the "or None" part was
silently discarded. It is now `Optional[dict]`, which is what the body
actually returns.

Added focused regression tests for both fixes. Note that the class still
cannot be re-opened after `close()` (a `threading.Thread` can only be
started once) even though `SourceReader`'s documented contract allows it —
that needs a larger refactor and is filed separately.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
@thorwhalen
thorwhalen merged commit b6ad4ab into master Aug 4, 2026
12 checks passed
@thorwhalen
thorwhalen deleted the claude/rollout-modernize branch August 4, 2026 13:39
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.

1 participant