Skip to content

Fix setuptools packaging - #860

Open
m-reuter wants to merge 5 commits into
Deep-MI:devfrom
m-reuter:fix-package
Open

Fix setuptools packaging#860
m-reuter wants to merge 5 commits into
Deep-MI:devfrom
m-reuter:fix-package

Conversation

@m-reuter

@m-reuter m-reuter commented Aug 30, 2026

Copy link
Copy Markdown
Member

pyproject.toml declared the installed packages as an explicit list:

[tool.setuptools]
packages = ['FastSurferCNN', 'CerebNet', 'recon_surf', 'HypVINN']

Two things were wrong with it.

1. CorpusCallosum was never added. It is a proper package (__init__.py plus seven subpackages), added after this list was written.

2. The list is not recursive. setuptools treats these as literal package names and does not descend into them, so no subpackage was installed either. A pip install . into a clean target gave:

site-packages/FastSurferCNN/ → __init__.py, download_checkpoints.py, inference.py,
                                run_prediction.py, segstats.py, ...  (no utils/, models/, config/)
site-packages/CerebNet/      → __init__.py, apply_warp.py, inference.py, run_prediction.py
site-packages/CorpusCallosum → absent

On top of that, no non-.py file was installed at all (0 data files), so even the modules that did land could not find checkpoint_paths.yaml, the network configs or the color lookup tables.

The installed distribution was therefore unable to import its own modules. This went unnoticed because nothing ever imports from site-packages: every entry point puts the source tree first on PYTHONPATH (run_fastsurfer.sh:576, recon_surf/long_prepare_template.sh:236, the Docker image's ENV PYTHONPATH=/fastsurfer, and the macOS package's postinstall). In practice pip install only served to resolve the dependency list.

Changes

[tool.setuptools.packages.find]
include = ['FastSurferCNN*', 'CerebNet*', 'CorpusCallosum*', 'HypVINN*', 'recon_surf*']

[tool.setuptools.package-data]
'*' = ['*.yaml', '*.tsv', '*.txt', '*.json', '*.csv']

Discovery is namespace-aware, so the directories that have no __init__.py are still picked up (FastSurferCNN/config, CerebNet/config/dataset, CorpusCallosum/config, recon_surf itself and recon_surf/utils) — nothing regresses versus the old explicit list. The include patterns keep doc, test, env, build and checkpoints out.

Also: pydocstyle checked nothing at all

While registering CorpusCallosum in [tool.pydocstyle], it turned out the existing setting was inert:

match-dir = '^FastSurferCNN.*,^CerebNet.*,^recon-surf.*,^HypVINN.*'

match-dir is a single regex matched against each directory name during the walk, not a comma-separated list. A directory is called FastSurferCNN or utils, never FastSurferCNN...,^CerebNet..., so the commas are literal characters and the embedded ^ anchors can never match. No directory matched at any level, the walk stopped at the repo root (which has no .py files), and pydocstyle . reported 0 files checked. Measured with pydocstyle 6.3.0.

An allow-list alternation would not fix it either, since the same pattern is applied at every level — ^(FastSurferCNN|CerebNet|...) would prune utils/, models/, config/ beneath them. So the packages are selected by excluding the non-package directories instead:

match-dir = '^(?!\.|__pycache__$|build$|checkpoints$|doc$|Documentation$|env$|test$|tools$|Tutorial$).*'

Traversal now covers exactly the 26 package directories — all five roots plus every subpackage, including CorpusCallosum/* and recon_surf/utils.

This surfaces 116 files with 513 existing docstring findings (previously 0/0). That is the pre-existing docstring backlog becoming visible, not new breakage, and it is why the pydocstyle step is commented out in .github/workflows/code-style.yml:34-35. CI is unaffected by this PR. The point of the change is that when the docstring cleanup is picked up later, pydocstyle . will actually report the work instead of silently returning clean.

Verification

pip install --no-deps --target into a scratch directory:

  • 26 packages installed (was 4): all five top-level packages and every subpackage, including CorpusCallosum and its seven subpackages.
  • 33 data files installed (was 0): all four checkpoint_paths.yaml, the FastSurferVINN_* / HypVINN_* / CerebNet_* configs, FastSurfer_ColorLUT.tsv, FreeSurferColorLUT.txt, HypVINN_ColorLUT.txt, CerebNet2FreeSurfer.json, fsaverage_target.json, the three DKTatlaslookup.txt.
  • Import smoke test run from a different working directory, so the source tree could not shadow the installed copy: FastSurferCNN.utils.checkpoint, CorpusCallosum.utils.checkpoint, HypVINN.models.networks, CerebNet.models.networks and recon_surf.image_io all import, and FASTSURFER_ROOT resolves inside the installed tree.

Impact

  • Docker: none. The Dockerfile uses pyproject.toml only for dependency resolution (uv pip compile --extra container ... | uv pip sync, tools/Docker/Dockerfile:159-172) and then COPY . /fastsurfer/ with PYTHONPATH=/fastsurfer. setuptools is never invoked, so these settings are inert there.
  • macOS package: the only place that actually builds a wheel (postinstall runs pip install "$FASTSURFER_HOME[qc]"). Its venv now gets a complete copy instead of a partial one. No behavioral change, since run_fastsurfer.sh prepends $FASTSURFER_HOME to PYTHONPATH either way.
  • CI: none. ruff and codespell are unaffected; pydocstyle remains disabled.

Groundwork for #262 / #618 (pip/conda distribution), but not a solution to them — those are blocked on the FreeSurfer dependency, not on this.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The pydocstyle directory expression matches none of the intended package directories.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes incomplete setuptools packaging as groundwork for pip/conda distribution.

Changes:

  • Enables recursive discovery of all five package trees.
  • Includes required runtime configuration and lookup data.
  • Updates pydocstyle directory matching.
File summaries
File Description
pyproject.toml Configures package discovery, package data, and pydocstyle scope.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pyproject.toml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The wheel still omits recon-surf.sh, which an installed Python module opens at runtime.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread pyproject.toml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The configuration correctly packages namespace subpackages and required runtime resources without affecting enabled CI checks.

Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

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.

2 participants