[trexfeathers]

(posted by Claude Opus)

Two rules for the benchmark README template, both learned by having the benchmark job fail in SciTools/iris, plus one line for the MANIFEST template. Nothing here changes behaviour in any repo on its own — it is guidance plus one packaging exclusion — so the risk is that the guidance is wrong or too narrow rather than that something breaks. I would value a check on whether the second rule generalises beyond the case that prompted it.

A benchmark run checks out each commit in a range and runs one copy of the benchmark code against all of them. Benchmark code that imports a name which has since moved therefore fails on every commit before the move, which is the whole before half of a before-and-after comparison. The same holds for a capability that did not always exist: a benchmark parametrised over it has nothing to time on the older commits, and attempting it reports a failure where there is no measurement to make. The README now says to import the current name with the old one as a fallback, and to raise NotImplementedError in setup() so ASV skips an unsupported parameter — leaving the parameter in params, since removing it renames the benchmark and ASV matches results by name.

prune .hooks is unrelated and much smaller: a repo keeping git hooks there was packaging them into the sdist. It sits with the other top-level directories that never belong in one.

Where I would look first

The NotImplementedError skip. It is ASV's documented mechanism and already used elsewhere in the Iris suite, but I have only exercised it for one case — a file format that older commits cannot write. If there is a kind of capability where skipping would hide a regression rather than avoid a meaningless measurement, the wording needs narrowing.

The generalised example in the import rule. I replaced the Iris module names with mypackage.formats.* so the template stays repo-neutral, which does make it more abstract than the real case.

What I rejected

Iris's MANIFEST.in has drifted from this template in several other places — global-exclude AGENTS.md and global-exclude CLAUDE.md, include environment.yml, and prune benchmarks in its repo-specific section. None of those came from the branch that prompted this, so none is mine to push back here; they are worth their own look by someone who knows why each was added. The template also still carries exclude .flake8, which Iris has dropped, and that is a question for the same review rather than this pull request.

Verification

The import rule and the skip were both run in SciTools/iris against two commits either side of the change that broke things: on the earlier commit the unsupported parameter reports n/a beside a measured one, and the benchmark that reaches the moved import runs where it previously raised ImportError. asv compare reports no false regression. The equivalent changes to the Iris copies of both files are in SciTools/iris#7354.

Iris already carries both of these changes, in SciTools/iris#7354, so it needs no issue raising against it when this merges: @scitools-templating: please no update notification on: iris

Every other repo using these templates should still be notified. MANIFEST.in is shared with seven more, and benchmarks/README.md with iris-esmf-regrid, and none of them has seen either change.