DiamondLightSource / DiamondLightSource/smartem-decisions

Remove setup.py by removing the need for per-package _version.py files

Open
#233 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Rewritten. The goal - deleting `setup.py` - is still right, but the original analysis contained
an error that would break the build if acted on, and the better route was not among the options
considered.

## Correction: `copy_version_files()` is not redundant, it is load-bearing

The original description states that `copy_version_files()` duplicates work already done by
setuptools-scm. It does not.

`setuptools_scm` is configured with a single target (`pyproject.toml`):

```toml
[tool.setuptools_scm]
version_file = "src/_version.py"
```

That generates **one** file. But three packages import their own version module:

```
src/__init__.py:1 from ._version import __version__ as __version__
src/smartem_backend/__init__.py:1 from ._version import __version__ as __version__
src/smartem_agent/__init__.py:1 from ._version import __version__ as __version__
```

And every `_version.py` is gitignored (`.gitignore:44`, `**/_version.py`), so none is committed.
The per-package files exist only because `copy_version_files()` creates them - confirmed by
`SOURCES.txt`, which lists `smartem_agent/_version.py`, `smartem_backend/_version.py` and
`smartem_common/_version.py` among the distribution contents, produced via the `egg_info` hook.

**Deleting `setup.py` as originally proposed would therefore break imports**, with
`ModuleNotFoundError: No module named 'smartem_agent._version'`.

The empirical evidence cited originally - that Docker builds produce correct `_version.py` files
without the hook firing - is consistent with this rather than contradicting it. #229 reworked
Docker to install from PyPI, and the published wheel already contains those files because the
hook ran during the release build. The observation was of the artefact, not the mechanism.

## The better route

Rather than replacing the copying mechanism, remove the need for it. Each package's
`__init__.py` can read the installed distribution's version directly:

```python
from importlib.metadata import version

__version__ = version("smartem-decisions")
```

This eliminates, in one change:

- the per-package `_version.py` copying, and therefore `copy_version_files()`
- the gitignored generated files scattered across packages
- the `cmdclass` hooks, which are a legacy mechanism unreliable under PEP 517 builds
- `setup.py` itself

`setuptools_scm` continues to determine the version at build time and stamp it into the
distribution metadata, which is what `importlib.metadata` then reads. Nothing about versioning
changes; only how packages find out what their version is.

Points to check when implementing:

- Behaviour when the package is not installed at all, which matters for running from a source
checkout without an editable install.
- Whether `src/_version.py` is still wanted for anything else before dropping the
`version_file` setting.
- Whether the agent's Windows executable build resolves distribution metadata correctly once
frozen - a bundled binary does not always carry package metadata, and this is the most likely
place for the approach to fail.

## The `.env` half

The original analysis of `copy_dotenv()` stands and needs no revision: it only fires reliably
for editable installs, does not run under PEP 517 wheel builds, and never enters the Docker
build context because `.dockerignore` excludes `.env.*`. It should be replaced with a
documented manual step or a Makefile target.

Also still worth resolving: whether the duplicate `.env.example` here still serves a purpose,
given the identical copy in `smartem-devtools/env-examples/`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.