Skip to content

Fix 3 dependency-analysis bugs, add pyproject.toml reading, modernize packaging + CI - #3

Merged
thorwhalen merged 10 commits into
masterfrom
wads/modernize-and-fix-dep-diff
Jul 30, 2026
Merged

Fix 3 dependency-analysis bugs, add pyproject.toml reading, modernize packaging + CI#3
thorwhalen merged 10 commits into
masterfrom
wads/modernize-and-fix-dep-diff

Conversation

@thorwhalen

Copy link
Copy Markdown
Member

Fixes three real code bugs, then does the full legacy modernization that was
blocked behind them. CI on master has been red on every run since at least
2025-10-15, so the publish job never ran and the last PyPI release is 0.1.10
from 2023-09-20.

Gate: 4 failed / 7 passed → 0 failed / 48 passed (verified on py3.11 and
py3.12, with the flags wads CI actually forces).

The three bugs

1. resolve_rootpath crashed on every non-package module

unbox/base.py read root.__path__ unguarded, so any module that is not a
package raised AttributeError. The fallback meant to handle exactly that case
was unreachable and itself broken — it did root = root.__file__ on a root
already known to be None.

User-facing consequence: imports_for, ModuleNamesImportedByModule and
everything built on them raised AttributeError: module 'wave' has no attribute '__path__' for any single-file module. Only packages worked.

Now resolves via __path__ when present, else __file__, and raises the
intended ValueError only when neither exists (namespace packages).

2. Pinned dependencies were all reported as missing

dependency_diff set-differenced the declared requirement strings against the
bare, import-derived names. Declared requirements carry PEP 508 version
specifiers, so 'dol>=0.3.49' never matched 'dol'.

User-facing consequence: this broke the library's headline feature. Every
pinned dependency of every analysed package was reported as both missing and
unused — i.e. print_missing_names / dependency_diff produced garbage for
essentially any real project. Measured on the pre-fix code:
dependency_diff(install_names=['dol>=0.3.49'], import_names=['dol']) returned
missing={'dol'}, unused={'dol>=0.3.49'}.

Declared requirements are now normalized to bare distribution names (versions,
markers, extras) before diffing. Deliberately not applied to
_parse_dependency_list / dependencies_from_setup_configs_content, which are
meant to round-trip the full specifier.

3. builtins was classified as third-party on py3.10

builtin_module_names was built only from the packaged
unbox/data/standard_lib_names/*.csv files. Those are machine-scanned
artifacts: 3.10.csv is missing builtins entirely, and the set stops at 3.10
(so every newer interpreter warns at import time and falls back to a slow
filesystem scan).

User-facing consequence: on py3.10, imports_for.third_party — and so every
"missing dependency" report — listed builtins. This was invisible on a py3.12
dev machine and failed only in CI, which is why it went unnoticed.

Now unions in the interpreter's own authoritative sys.stdlib_module_names
(py3.10+), so classification is correct on any version regardless of the
packaged CSVs.

Reading pyproject.toml — why the migration required it

unbox could only discover declared dependencies from setup.cfg
('setup.cfg' was hardcoded) and had zero PEP 621 support. So
find_install_names raised ValueError: Can't find install names for
essentially every modern package in the ecosystem — a dependency-analysis tool
that cannot read pyproject.toml is itself defective.

It was also a hard blocker for this repo's own migration: three of unbox's
doctests analyse unbox, so deleting setup.cfg without this support first
would have broken them.

  • _get_project_file_path(x, filename) generalizes the path lookup.
    get_setupcfg_path stays as a thin wrapper with unchanged behaviour;
    get_pyproject_path is its new sibling. Pointing either at the other
    project file now resolves rather than failing, so the fallback chain works on
    any project-root-ish input.
  • dependencies_from_pyproject_content(content, *, extras=False) reads
    [project] dependencies and, opt-in, [project.optional-dependencies]
    (extras=True for all groups, or an iterable of group names). Required-only
    by default, so existing semantics are preserved. [build-system] requires
    are correctly not treated as dependencies.
  • module_requirements_according_to_pyproject mirrors
    module_requirements_according_to_setupcfg (returns None when absent).
  • find_install_names tries pyproject.toml then setup.cfg, via an injectable
    finders argument (DFLT_INSTALL_NAMES_FINDERS).

Kept proportionate: standard PEP 621 locations only, no general TOML config
framework. Scope is limited to reading; nothing writes TOML.

Everything above is additive — no public name changed meaning, and legacy
setup.cfg-only projects keep working (pinned by a test).

Modernization

  • Packaging: setup.py + setup.cfg → hatchling pyproject.toml. SPDX
    license = "Apache-2.0" (was the non-conforming apache-2.0), plus trove
    classifiers, keywords, author and a Documentation URL — PyPI previously showed
    zero classifiers. Dropped the vestigial [options.data_files], which
    installed unbox/data/* to a top-level data/ prefix (the data is actually
    read via importlib.resources). Verified empirically that all 7
    standard_lib_names CSVs and dflt_import_to_install_name_map.json are
    present in both the built wheel and sdist, and that tests/.DS_Store are not.
    Wheels are now built and published, so PyPI will get real Requires-Dist
    metadata for the first time (previous releases were sdist-only).
  • CI: the pre-uv workflow (actions/checkout@v2, axblack, pylint C0114,
    isee install-requires, twine, pack check-in) → the wads uv
    reusable-workflow stub, configured from [tool.wads.ci.*]. No repo secrets
    needed; the i2mint org already provides PYPI_PASSWORD/PYPI_USERNAME with
    ALL visibility.
  • .editorconfig added; [tool.ruff] config added with a narrow
    select = ["D100"] so the repo is not exposed to ruff default-ruleset drift.
  • ruff format applied in its own commit, since the new CI runs it
    unconditionally before the tests — better to review the reformat here than
    to have it land inside a release commit.
  • Portability: PosixPathPath (PosixPath(...) raises
    NotImplementedError on Windows). test_on_windows stays false until a
    Windows leg has actually been observed green.

Regression tests

The repo had zero test files; 11 doctests were the only coverage. All three bugs
were interpreter-version-sensitive, so tests/test_regressions.py pins each one
explicitly and environment-independently (32 tests). Confirmed they genuinely
fail against the pre-fix code.

Note the tests live in a repo-root tests/, deliberately not inside the
package. An in-package tests/ was tried first and broke two other
self-referential doctests: pytest became one of unbox's own imports (reported
as a falsely-missing install name), and the file listing asserted by
unbox.recipes.key_and_pattern_counts changed. Both were observed, not
theorized.

Notes for review

  • requires-python = ">=3.11", narrower than the fleet's usual >=3.10.
    Rationale: the pyproject reader uses the stdlib tomllib (3.11+). The tomli
    backport was rejected because it would add a third-party import name that
    unbox's own self-referential doctests then report as a missing dependency.
    Declaring >=3.10 while the new feature silently has no TOML parser there
    would be dishonest, and py3.10 reaches EOL in October 2026. The CI python
    matrix matches (3.11, 3.12).
  • The six dependencies are carried over verbatim, deliberately. unbox's
    doctests diff this list against unbox's own imports, so both the apparently
    redundant importlib_resources and the deprecated py2store (still a real
    import in recipes.py) must stay declared. Please don't "clean up" this list
    without re-running the doctests.
  • pages-build-deployment will keep failing on every push. Pages serves
    master:/docs, but docs/ was deleted in c425dd8 and is gitignored. Fixing
    it means switching the Pages source to gh-pages / (root), which is a
    maintainer decision and is deliberately not part of this PR. Pre-existing
    and non-gating — it does not affect Continuous Integration.
  • The Publish job is skipped on non-default branches, as expected. Merging
    this will be the first publish since 2023-09-20.
  • origin/fix/issue18-wrapped-self (8d2a5b5) is superseded — its
    unbox/base.py is byte-identical to master's, which already has the change as
    squashed PR Fix dol Issue #18 in ModuleNamesImportedByModule.print_kvs #2. Safe to delete; not touched here.
  • Latent issue left out of scope: imports_for scans a package's entire source
    tree, so any project with tests inside its package will have its test-only
    imports (pytest, ...) reported as missing runtime dependencies. unbox has no
    notion of dev/test-only imports. Worth a follow-up issue.

https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475

`resolve_rootpath` read `root.__path__` unconditionally, so it raised
`AttributeError` for every plain (non-package) module -- e.g.
`import wave; imports_for(wave)`. The `if root is None:` fallback meant to
handle that case was both unreachable and itself broken: it did
`root = root.__file__` on a `root` already known to be `None`.

Resolve via `__path__` when present, else `__file__`, and raise the
intended `ValueError` only when neither exists (namespace packages).

Also make the `imports_for` doctest version-stable: it pinned py3.10's
`wave` imports (`audioop`/`chunk`, both removed in 3.13; py3.12 imports
`uuid` instead), so it could only ever pass on one interpreter.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
`builtin_module_names` was built only from the packaged
`unbox/data/standard_lib_names/*.csv` files. Those are machine-scanned
artifacts: `3.10.csv` is missing `builtins` entirely, and the set stops at
3.10 (so every newer interpreter warns at import time and falls back to a
slow filesystem scan of the stdlib dir).

Consequence: on py3.10 `builtins` was classified as a third-party package,
so `imports_for.third_party` -- and therefore every "missing dependency"
report -- listed `builtins`. This was invisible on a py3.12 dev machine and
only showed up in CI.

Union in the interpreter's own authoritative `sys.stdlib_module_names`
(py3.10+) so classification is correct on any version, packaged CSV or not.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
`dependency_diff` set-differenced the *declared* requirement strings against
the bare, import-derived distribution names. Declared requirements carry
version specifiers and markers, so `'dol>=0.3.49'` never matched `'dol'`.

Consequence: every pinned dependency of every analysed package was reported
as a missing install name (and simultaneously as an unused one). That breaks
the library's headline feature -- `print_missing_names` / `dependency_diff`
were unusable on any project that pins a version, which is nearly all of
them.

Normalize declared requirements to bare distribution names via the new
`_dist_name` helper before diffing. Deliberately NOT applied to
`_parse_dependency_list` / `dependencies_from_setup_configs_content`: those
are meant to round-trip the full specifier and their doctests assert it.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
unbox could only discover a package's declared dependencies from
`setup.cfg` -- `'setup.cfg'` was hardcoded in `get_setupcfg_path` and there
was no PEP 621 support at all. So `find_install_names` raised
`ValueError: Can't find install names` for essentially every modern package
in the ecosystem, making `dependency_diff` / `print_missing_names` unusable
on them. A dependency-analysis tool that cannot read pyproject.toml is
itself defective.

It is also the prerequisite for this repo's own migration: unbox's doctests
analyse unbox, so deleting setup.cfg without this support would break them.

- Generalize the path lookup into `_get_project_file_path(x, filename)`.
  `get_setupcfg_path` stays as a thin wrapper with unchanged behaviour;
  `get_pyproject_path` is its new sibling. Pointing either at the *other*
  project file now resolves rather than failing, so the fallback chain works
  on any project-root-ish input.
- Add `dependencies_from_pyproject_content(content, *, extras=False)`,
  reading `[project] dependencies` and, opt-in, `[project.optional-dependencies]`
  (`extras=True` for all groups, or an iterable of group names). Required-only
  by default, so the conservative existing semantics are preserved.
- Add `module_requirements_according_to_pyproject`, mirroring
  `module_requirements_according_to_setupcfg` (returns None when absent).
- `find_install_names` now tries pyproject.toml then setup.cfg, via an
  injectable `finders` argument (`DFLT_INSTALL_NAMES_FINDERS`).
- Use `pathlib.Path` instead of `PosixPath`, which raises
  NotImplementedError on Windows.

TOML is parsed with the stdlib `tomllib` only. The `tomli` backport is
deliberately avoided: it would add a third-party import name that unbox's
own self-referential doctests would then report. Hence requires-python >=3.11.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
setup.cfg was the source of truth (setup.py was a 3-line shim). Replaced by
a hatchling-backed pyproject.toml, in the fleet-standard shape.

Fixes carried over from the legacy metadata:
- SPDX `license = "Apache-2.0"` instead of the non-conforming `apache-2.0`.
- Trove classifiers, keywords, author and Documentation URL, all previously
  absent (PyPI showed zero classifiers).
- Dropped `[options.data_files]`, which installed unbox/data/* to a top-level
  `data/` prefix. It was vestigial and wrong -- the data is read via
  `importlib.resources.files('unbox')/'data'`. Verified empirically that all
  7 standard_lib_names CSVs and dflt_import_to_install_name_map.json are
  present in both the built wheel and sdist.
- Wheels will now be built and published, so PyPI gets real `Requires-Dist`
  metadata for the first time (previous releases were sdist-only).

`requires-python = ">=3.11"`, narrower than the fleet's usual >=3.10, so the
pyproject reader can use the stdlib `tomllib`; the CI python matrix matches
(3.11, 3.12). `testpaths = ["unbox"]` is required because wads' test action
passes no path to pytest. `test_on_windows` stays false until a Windows leg
has actually been observed green.

The six dependencies are carried over verbatim, deliberately: unbox's own
doctests diff this list against unbox's imports, so both the apparently
redundant `importlib_resources` and the deprecated `py2store` (still a real
import in recipes.py) must stay declared.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
The old workflow was the pre-uv wads generation: actions/checkout@v2 +
actions/setup-python@v2 (both emitting Node.js 20 deprecation warnings),
axblack formatting, `pylint --enable=C0114`, `isee install-requires`,
`python setup.py sdist` + twine, `pack check-in`, `isee tag-repo`.

Replaced with the stub that calls i2mint/wads/.github/workflows/uv-ci.yml,
so all configuration now lives in pyproject.toml [tool.wads.ci.*]. The old
`env: PROJECT_NAME` is dropped -- the project name comes from pyproject now.

No repo secrets are needed: the i2mint org already provides PYPI_PASSWORD
and PYPI_USERNAME with ALL visibility, and the stub's explicit secrets
pass-through resolves them.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
Standard wads template: utf-8 / lf / final newline / trimmed trailing
whitespace, 4-space indent for py/toml/yml.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
The 3 bugs just fixed were all interpreter-version-sensitive (two presented
differently on py3.10 vs py3.12, one was invisible locally and failed only in
CI), and doctests were the repo's only tests. These are explicit,
environment-independent pins so the bugs cannot silently come back:

- resolve_rootpath on a non-package module / a package / a module with no
  __file__, plus imports_for on a non-package module (the original symptom).
- _dist_name specifier stripping (versions, markers, extras) and
  dependency_diff both matching pinned deps AND still reporting genuinely
  missing/unused ones.
- stdlib names (including `builtins`) classified as builtin on any python
  version, and imports_for.third_party never returning a stdlib name.
- the new pyproject.toml reader: pyproject-only projects, setup.cfg-only
  projects (back-compat), precedence, opt-in extras, symmetric path lookup.

Verified these fail against the pre-fix code: bug 1 raises
`AttributeError: module 'dataclasses' has no attribute '__path__'`, and bug 2
yields `missing={'dol'} unused={'dol>=0.3.49'}`.

Tests live in a repo-root `tests/`, deliberately NOT inside the package:
unbox analyses its own source tree in its doctests, so an in-package tests dir
adds `pytest` to unbox's own imports (reported as a falsely-missing install
name) and changes the file listing asserted by
`unbox.recipes.key_and_pattern_counts`. Both were observed empirically.
`testpaths` is extended so wads' CI (which passes no path to pytest) still
collects them.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
The codebase was formatted with axblack (single quotes); the new CI runs
`uvx ruff format .` unconditionally *before* the tests, so applying it here
keeps that step a no-op and lets the reformat be reviewed on its own rather
than landing inside a release commit. Purely mechanical (ruff 0.16.0) --
verified `ruff check unbox` clean and the full suite green on py3.11 and
py3.12 after the reformat.

Note ruff also formats python code blocks inside README.md.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
The `imports_for(wave)` example asserted py3.10's exact import set
(`audioop`/`chunk`, both removed in 3.13), so it was already false on any
modern interpreter -- the same bug class as the doctest fixed earlier. Assert
a version-stable subset instead, and say why.

Also document that declared dependencies are now read from pyproject.toml
(with the setup.cfg fallback) and how to opt into extras.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
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