Regroup every module into its layer directory - #173
Merged
Merged
Conversation
The six layers the dependency rules already enforced are now directories, so
a module's location IS its layer:
presentation/ cli/, auto3Dcli
entry/ auto3D, SPE, ASE/, tautomer
orchestration/ workflow, workflow_workers, chunk_manager, job_layout,
processors, pipeline/
engines/ models/, model_factory, isomers/, batch_opt/
domain/ ranking, filtering, embedding, clash_relief, id_mapping
foundation/ config, constants, exceptions, registry, results,
torch_config, utils/
79 modules moved; 175 files rewritten. Layer packages are namespacing only
and re-export nothing, per the barrel rule.
WHAT THIS BUYS. The layer map in tests/test_layer_boundaries.py was a
per-module prefix list that had to be edited on every move and could go
stale in BOTH directions: a module in no layer (caught from the start) and a
layer prefix naming no module (uncaught until #169 added a test for it,
after #168 left `isomer_engine` behind). Both failures are now
inexpressible. The map is derived from directory names; the only thing still
declared is the ORDER of the layers, which is the one fact a directory
cannot carry. The staleness test shrank to "does every layer directory
exist" -- a layer can no longer be partly stale.
THE TOP-LEVEL API IS UNAFFECTED. `from Auto3D import main, smiles2mols,
Auto3DOptions, calc_spe, calc_thermo, opt_geometry, create_model, ...` all
still work: the package root re-exports them lazily through `_LAZY_API` and
its contents did not change. Only module paths move, so code importing from
`Auto3D` directly is untouched. This is what turns the change from "every
user's imports break" into "code reaching into submodules breaks", and it
rests on one dictionary -- exercised by test_public_api.py and by the api.rst
resolution test, not merely eyeballed.
Breaking for module paths, with a CHANGELOG table. The exception classes are
the ones most likely to appear downstream: all eight moved to
`Auto3D.foundation.exceptions`. No shims -- a shim would give every moved
name two supported spellings, which is the rule the barrels already state.
FIVE KINDS OF REFERENCE A DOTTED-PATH REWRITE DOES NOT FIND, all hit here:
* `from Auto3D import model_factory` -- a submodule imported by NAME. The
text `Auto3D.model_factory` never appears. 10 sites, 5 files.
* filesystem fragments -- `_submodules("utils")`, `SRC_ROOT / "isomers" /
...`, and a checkpoint path a test reads off disk.
* assertion literals -- `["Auto3D/isomers/base.py"]` as an expected value.
* `[project.scripts]` -- the `auto3d` console script entry point. NO test
covers it; the suite drives the CLI through Typer's runner.
* `[tool.setuptools.package-data]` -- `models/*.pt`. The wheel would have
shipped without the ANI2xt checkpoint. Tests pass from the source tree
either way, so nothing would have failed until someone installed it.
The last two are the ones worth remembering: a green suite says nothing
about either.
DOCUMENTATION. api.rst and the 25 checked-in autosummary stubs were renamed
and rewritten (Sphinx keys them by dotted path; titles and underlines
included). The current-usage docs were rewritten. The two migration guides
were NOT: both are before/after records -- migration.rst's "before" is v2.x
code -- so rewriting their "after" halves would make each describe a layout
that did not exist when the change it documents was made. Each carries a
note saying its paths are era-specific, pointing at the CHANGELOG table.
Verification:
- 1771 passed, 1 skipped, 74 deselected -- the same count as before the
move, randomized order.
- mypy: 68 errors, unchanged; 85 files checked instead of 79, which is the
six new layer __init__.py files.
- test_no_module_imports_a_higher_layer still passes, so the layering that
was enforced against the old map holds against the derived one.
CI caught a sixth kind of reference the layer move has to update, and it is
not in any file the earlier sweeps looked at:
* `.github/workflows/tests.yml` -- a dotted import inside a heredoc. The
"Warm the AIMNet2 model cache" step runs
`from Auto3D.model_factory import create_model` in a `python - <<'PY'`
block, so it is neither Python source nor documentation and neither
scan covered it. It failed the slow tier in 1m48s with
ModuleNotFoundError, before a single test ran.
* `conda-recipe/meta.yaml` -- the conda entry point, the same
`Auto3D.auto3Dcli:cli` string already fixed in `[project.scripts]`.
Nothing in CI builds the conda package, so this one would have shipped
a broken `auto3d` command to conda users with every gate green.
* `CONTRIBUTING.md` -- live developer docs, rewritten.
`pyproject.toml`'s filterwarnings entries (`Auto3D.*`) are deliberately
untouched: that field is a regex over the module name, and `Auto3D.*` still
matches every submodule under the new layout.
`CHANGELOG.md`'s older entries keep their old paths, for the same reason the
two migration guides do -- they describe what was true in a past release, and
rewriting them would misdate the layout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The six layers the dependency rules already enforced are now directories, so a module's location is its layer:
79 modules moved, 175 files rewritten. Layer packages are namespacing only and re-export nothing, per the barrel rule.
What this buys
The layer map in
tests/test_layer_boundaries.pywas a per-module prefix list that had to be edited on every move, and could go stale in both directions: a module in no layer (caught from the start) and a layer prefix naming no module (uncaught until #169 added a test for it — after #168 leftisomer_enginebehind).Both failures are now inexpressible. The map is derived from directory names, and the only thing still declared is the order of the layers, which is the one fact a directory name cannot carry. The staleness test shrank to "does every layer directory exist" — a layer can no longer be partly stale.
The top-level API is unaffected
from Auto3D import main, smiles2mols, Auto3DOptions, calc_spe, calc_thermo, opt_geometry, create_model, ...all still work. The package root re-exports them lazily through_LAZY_APIand its contents did not change.That is what turns this from "every user's imports break" into "code reaching into submodules breaks" — and it rests on one dictionary, exercised by
test_public_api.pyand by the api.rst path-resolution test rather than merely eyeballed.Breaking for module paths, with a CHANGELOG table. The exception classes are the ones most likely to appear downstream: all eight moved to
Auto3D.foundation.exceptions. No shims — a shim would give every moved name two supported spellings, which is the rule the barrels already state.Five kinds of reference a dotted-path rewrite does not find
All five hit here, and they're the transferable part of this PR:
from Auto3D import model_factory— the textAuto3D.model_factorynever appears (10 sites, 5 files)_submodules("utils"),SRC_ROOT / "isomers" / ..., a checkpoint path read off disk["Auto3D/isomers/base.py"]as an expected value[project.scripts]auto3dconsole-script entry point[tool.setuptools.package-data]models/*.pt— the wheel would have shipped without the ANI2xt checkpointThe last two matter most: tests pass from the source tree regardless, so a green suite says nothing about either. I found them by reading
pyproject.toml, not by running anything.Documentation
api.rstand the 25 checked-in autosummary stubs were renamed and rewritten — Sphinx keys them by dotted path, so titles and underlines moved too. The current-usage docs were rewritten.The two migration guides were deliberately not. Both are before/after records —
migration.rst's "before" is v2.x code — so rewriting their "after" halves would make each describe a layout that did not exist when the change it documents was made. Each now carries a note saying its paths are era-specific and pointing at the CHANGELOG table.Verification
__init__.pyfiles.test_no_module_imports_a_higher_layerstill passes, so the layering that was enforced against the old declared map holds against the derived one.