Skip to content

move the package to a src/ layout so tests exercise the installed package #345

Description

@dave-doty

The package directory scadnano/ sits at the repository root (a "flat" layout). That means the repo root is itself importable as the package, so import scadnano from the root finds the working tree, not whatever is installed. Nothing in CI currently tests the artifact users actually receive.

Concretely, run_unit_tests.yml runs:

python -m unittest -v tests/scadnano_tests.py

from the repo root. sys.path[0] is the repo root, so import scadnano as sc at tests/scadnano_tests.py:13 resolves to ./scadnano/. Installing the package first — which the workflow now does, via pip install -e .[tests] — does not change that. The tests pass whether or not the wheel is correct.

Moving the package to src/scadnano/ fixes this structurally: the repo root stops being importable as the package, so the only way to import it is from an install.

What this would catch

A wheel missing a module, or a module present in the tree but excluded from [tool.setuptools] packages. That class of bug ships green today. It is not hypothetical for this project — the recent packaging work turned up two neighbours of it:

  • scadnano.__version__ was not reachable for an installed user at all, because scadnano/__init__.py uses from scadnano.scadnano import * and import * skips underscore-prefixed names. Every test passed throughout, because tests import from the tree.
  • 21 example scripts imported origami_rectangle and modifications as top-level modules. That only resolves when scadnano/ itself is on sys.path — it was broken for every pip-installed user, and no test noticed.

Partial mitigation already in place

check_pypi_packaging.yml now installs the built wheel into a clean virtualenv and imports it from a directory containing no source tree, asserting scadnano.__version__ matches pyproject.toml and that a design serializes. That covers the "is the artifact importable and correctly versioned" case, and it is why this is an improvement rather than a gap.

What it does not cover is the full test suite running against the installed package. Only a layout change gets that.

Sketch

  1. git mv scadnano src/scadnano.
  2. pyproject.toml: replace [tool.setuptools] packages = ["scadnano"] with [tool.setuptools.packages.find] where = ["src"].
  3. doc/conf.py: drop sys.path.insert(0, os.path.abspath('../scadnano')) and rely on the installed package. This is the fiddly part — see below.
  4. run_unit_tests.yml: pip install .[tests], non-editable, so the installed copy is what gets imported.
  5. Check the helper scripts and .gitignore for hardcoded scadnano/ paths.

The one genuinely awkward piece

doc/conf.py currently inserts ../scadnano onto sys.path, which makes scadnano resolve to the module scadnano.py rather than the package. doc/index.rst depends on this: .. automodule:: scadnano documents that module's members, and .. automodule:: origami_rectangle only resolves because the package directory is on the path.

Under a src/ layout, automodule:: scadnano would document the package, whose members arrive via from scadnano.scadnano import *. autodoc skips imported members by default (they carry __module__ == "scadnano.scadnano"), so a naive switch likely produces empty API documentation — and readthedocs.yml sets fail_on_warning: true, so the failure mode is at least loud rather than silent.

Resolving that means deciding what the documented module path should be — scadnano.Design or scadnano.scadnano.Design — and possibly adding __all__ or :imported-members:. It also changes anchors on the published docs for anything under origami_rectangle, which the README links to. Worth settling before starting, since it is the part most likely to derail the change.

Cost

Mostly mechanical, but it touches the import path every contributor types, rewrites all doc anchors under origami_rectangle, and invalidates any local checkout's muscle memory. Fine to defer; the wheel smoke test above buys most of the safety in the meantime.

No activity

Activity on this issue will appear here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions