Skip to content

Refactor into an installable pyquiver package with a modern Python API - #8

Merged
ekwan merged 14 commits into
masterfrom
refactor/package-and-api
Jun 17, 2026
Merged

ekwan merged 14 commits into
masterfrom
refactor/package-and-api

Conversation

@ekwan

@ekwan ekwan commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Turns the flat src/ scripts into an installable, importable package (pyquiver, distributed as pyquiver-kie) with a modern, data-returning Python API — while preserving all numerical results (locked by a new regression suite) and keeping old imports working via shims.

Highlights

Packaging & tooling

  • pyquiver/ package + pyproject.toml (PyPI name pyquiver-kie, import name pyquiver), weights.dat as package data, optional [pandas]/[dev] extras, pyquiver console script
  • GitHub Actions: CI (uv, Python 3.9–3.13, build + twine check) and PyPI publish via Trusted Publishing
  • back-compat shims in src/ so import quiver / from kie import ... still work (with a deprecation warning)

Library hygiene

  • raises exceptions instead of sys.exit/print; diagnostics via logging; silent import
  • parsing split into pyquiver/parsers/{gaussian,orca,native} behind a style registry — gaussian is the preferred name (g16/g09 aliases); one shared periodic table
  • dead code removed (utility.py, dump_debug, …)

API features

  • structured results: KIEResult/EIEResult, Results, to_dict/to_csv/to_dataframe
  • Config.from_dict — build configurations in code, no .config file
  • polymorphic isotopic replacement: a weights.dat label or a bare numeric mass (unusual isotopes need no label)
  • optional thread parallelism (n_jobs; eigvalsh releases the GIL)
  • Skodje-Truhlar correction (pyquiver.tunneling + KIE_Calculation.skodje_truhlar)
  • batch(config, pairs, ...) -> BatchResults replaces autoquiver and its CLI; pairs as dict / triples / (gs, ts) tuples; optional energies adds a batch Skodje-Truhlar column
  • the pyquiver command line is deprecated in favour of the Python API

Bug fixes found along the way
.qin trailing-comma parser, missing numpy import, a %-format string, the Config-object constructor branch, an eie_flag NameError, a get_row/__str__ mass-override crash, a missing return in KIE.__str__, the autoquiver match-predicate checking the wrong variable, an autoquiver cwd leak, and AttributeError→ValueError for malformed Gaussian input.

Tests & docs

  • pytest suite: 189 tests, 100% coverage (pragmas only on genuinely unreachable defensive branches)
  • README/DETAILS/AUTOQUIVER/TUTORIAL build the config in Python; rebuilt scripts/pyquiver.ipynb (supported file types, config in Python, batch, ORCA, tunnelling equations, RPFR diagnostics) and removed the standalone Skodje-Truhlar notebook (folded into the tunnelling section)

Notes

  • Publishing stays dormant until a GitHub Release is created (the pending Trusted Publisher will match publish.yml / environment pypi).
  • Confirm the pyquiver-kie name on PyPI before the first release.

🤖 Generated with Claude Code

ekwan and others added 14 commits June 17, 2026 07:17
Restructure the flat src/ scripts into an installable, importable package
and modernize the API, while preserving all numerical results (locked by a
new regression suite) and backward-compatible imports.

Packaging & tooling
- move src/*.py into a pyquiver/ package; pyproject.toml (PyPI name
  pyquiver-kie, import name pyquiver), console script `pyquiver`,
  weights.dat as package data, optional [pandas]/[dev] extras
- GitHub Actions: CI (uv, py3.9-3.13, build + twine check) and PyPI
  publish via Trusted Publishing
- back-compat shims in src/ for old `import quiver`/`from kie import ...`

Library hygiene
- raise exceptions instead of sys.exit/print; route diagnostics through
  logging; silent import; remove dead code (utility.py, dump_debug, etc.)
- split parsing into pyquiver/parsers/{gaussian,orca,native} behind a style
  registry; `gaussian` is the preferred style (g16/g09 aliases); one shared
  periodic table

API features
- structured results: KIEResult/EIEResult, Results, to_dict/to_csv/to_dataframe
- Config.from_dict for building configs in code (no .config file)
- polymorphic isotopic replacement: a weights.dat label OR a numeric mass
- optional thread parallelism (n_jobs; eigvalsh releases the GIL)
- Skodje-Truhlar correction (pyquiver.tunneling + KIE_Calculation.skodje_truhlar)
- batch(config, pairs, ...) -> BatchResults (replaces autoquiver + its CLI);
  pairs as dict/triples/2-tuples; optional energies for batch Skodje-Truhlar
- deprecate the `pyquiver` command line in favour of the Python API

Bug fixes found along the way
- .qin trailing-comma parser, missing numpy import, config % format string,
  KIE_Calculation Config-object branch, eie_flag NameError, get_row/__str__
  mass-override crash, KIE.__str__ missing return, autoquiver match-predicate
  checking the wrong variable, autoquiver cwd leak, gaussian AttributeError

Tests & docs
- pytest suite, 189 tests, 100% coverage (pragmas only on unreachable
  defensive branches)
- README/DETAILS/AUTOQUIVER/TUTORIAL build the config in Python; rebuilt
  scripts/pyquiver.ipynb (supported file types, config in Python, batch,
  ORCA, tunnelling equations, RPFR diagnostics; folds in the deleted ST notebook)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI: the test job used `uv pip install --system`, which uv refuses against
its managed Python ("externally managed"). Install into a venv with
`uv sync --extra dev --extra pandas` and run via `uv run --no-sync`; the
build job now uses `uv build` + `uvx twine check`.

Notebook (scripts/pyquiver.ipynb), per review:
- all imports in one cell at the top (incl. pandas and cctk)
- intro explains what a KIE calculation is
- simpler "simple calculation"; double substitution (H/D) called out and the
  numeric-mass option noted inline; per-style parsing folded in (no separate
  "supported files"/"ORCA" sections)
- the three batch pair forms each in their own beginner-friendly cell
- separate Wigner, Bell, and Skodje-Truhlar equation cells (ST reproduced
  from the original notebook); Bell inverted parabola noted as the default
  and guidance on when to use Skodje-Truhlar
- RPFR per-mode contributions shown as data, not a debug log
- executed so outputs are embedded (no need to re-run to read it)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Read the tutorial notebook from a new user's perspective (not assumed to be
Python or KIE experts) and reworked the markdown: define ground/transition
state, isotopologue, the config parameters (temperature/scaling/imag_threshold),
tunnelling, and RPFRs in plain language; smoother transitions; no em dashes;
mark the deep-dive sections optional. Code and embedded outputs unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The old DEBUG log printed each mode's light/heavy frequency, frequency ratio,
ZPE factor, and contribution. The notebook's partition_components table had
dropped the frequencies; add light_freq/heavy_freq columns back (and keep the
excitation factor), so the data view matches and exceeds the old log.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code fixes:
- the `pyquiver` console script printed the table twice and exited 1
  (setuptools wraps the entry point in sys.exit(), which re-printed main()'s
  return value); point the script at a _console() wrapper that returns None
- Config.__str__ raised ValueError when a replacement was a numeric mass;
  format it with !s
- structured results (Results / to_dict / to_csv / to_dataframe and batch) now
  exclude the reference isotopologue, matching the printed table and get_row
  (it had been listed with its absolute value among the referenced ones)
- Config.from_dict coerces numeric-string replacements ("5000.0") like the
  .config file parser

Notebook:
- fixed the swapped calculate_rpfr unpacking (heavy is index 2, light is
  index 3) so the diagnostic light/heavy frequency columns are labelled
  correctly and the per-mode factors multiply to the RPFR; clarified that the
  Skodje-Truhlar nu is in Hz; re-executed so embedded outputs are correct

Docs:
- TUTORIAL: refreshed stale KIE numbers, -v -> -vv for the config echo,
  src/weights.dat -> pyquiver/weights.dat, typo
- README: to_dataframe column is `name` (not isotopologue); softened the
  "no positional indexing" claim; removed em dashes
- DETAILS: Skodje spelling, page range 624-626, "Rearrangement", typos
- SNIP/AUTOQUIVER: grammar/typo and an em dash

Tests (193, 100% coverage): pin calculate_rpfr return order; validate the
Skodje-Truhlar referencing arithmetic; Config.__str__ with a numeric mass;
from_dict numeric-string coercion; results exclude the reference.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both regimes give unreliable tunnelling results, silently before.

- skodje_truhlar now warns when the temperature is below the crossover
  (alpha < beta) for either imaginary mode, where the inverted-parabola model
  diverges.
- KIE_Calculation warns when an isotopologue looks like a *primary* hydrogen
  KIE: a hydrogen (Z=1) substitution whose transition-state imaginary-frequency
  isotope ratio exceeds a threshold (the H sits in the reaction coordinate),
  where the harmonic Wigner/Bell corrections are unreliable. Secondary H KIEs
  (small imaginary ratio, e.g. the Claisen H/D) do not warn.

Each KIE now stores ts_imag_ratio (the raw light/heavy TS imaginary ratio).
Tests cover both warnings (and the no-warn cases); notebook notes the caveats.
197 tests, 100% coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the imaginary-frequency-ratio proxy with a more direct test: how much
the substituted hydrogen participates in the transition-state reaction mode.

Use the mass-weighted mode composition (per-atom q^2 of the most-negative
eigenvector), not raw Cartesian displacement -- Cartesian displacement is
mass-amplified, so a light H always looks large (e.g. 0.15 for the secondary
Claisen H/D) and fails to discriminate. The mass-weighted participation gives
the Claisen H/D only 0.017 (correctly secondary) while the heavy C/O atoms
carry the mode; a transferring (primary) H would carry a large fraction.

- Isotopologue.reaction_mode_composition(): per-atom mass-weighted fraction of
  the reaction mode (memoised; one eigh on the TS).
- KIE_Calculation warns when a substituted hydrogen's fraction exceeds
  PRIMARY_HYDROGEN_MODE_FRACTION (0.1).

197 tests, 100% coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make clear that for a primary H/D KIE the predicted value may be unreliable
because tunnelling is large (not merely that the corrections are approximate).
Notebook note updated to match.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Acting on three reviews (science, pedagogy, rendering):

Rendering (the "doesn't render" cause): replace plain-TeX {a \over b} with
\frac{}{} and \ddag with \ddagger throughout, and put display equations in
their own blocks. These render in JupyterLab but break on GitHub/nbviewer.

Pedagogy: example-driven and less verbose. First calculation is a single
substitution; results-as-data follows immediately; multi-atom and custom-mass
substitutions are introduced when first used; the batch "three ways" collapse
to one example plus a sentence; tunnelling prose is trimmed. Added chemical
interpretation of the numbers (normal vs inverse KIEs).

Science: KIEs framed as a ZPE/free-energy effect (the electronic barrier is
unchanged); softened "Bell is the default" for heavy-atom KIEs; primary-H
caveat phrased as the harmonic treatment being approximate; noted the KIE is
RPFR_GS/RPFR_TS times the imaginary-frequency ratio and tunnelling; renamed the
diagnostic "product" column to "freq_ratio". Re-executed; outputs embedded.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keep all the math: re-add the natural-frequency definition and the alpha=beta
case alongside the alpha>beta and alpha<beta forms (trimmed in the rewrite).
Still uses GitHub-safe \frac and \ddagger. Markdown-only; outputs unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
batch now takes one form, a {label: (gs, ts)} dictionary (the Pythonic "one
obvious way"); the (label, gs, ts) triple and bare (gs, ts) forms are removed.
You build the dict from disk yourself, e.g. with a comprehension over a glob
and a lambda to generate labels. Tests and docs (README, AUTOQUIVER) updated.

Notebook: rewritten background and "Hello Isotope" sections; batch section now
shows the dict form plus building it from files with a label lambda; re-executed
so outputs are embedded.

195 tests, 100% coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The glob example labelled the run "claisen" (the molecule), which is pointless.
Batch's purpose is screening, so the label should be the axis you vary (the
level of theory). Keep one runnable dict example labelled b3lyp/m06, and make
the build-from-disk version an illustrative snippet that derives the level of
theory from filenames (b3lyp_gs.out -> b3lyp) with a lambda.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bell's kappa = (u/2)/sin(u/2) is the correction for an *infinite* (untruncated)
inverted parabolic barrier, conventionally the "infinite parabola"; that is the
precise name, and it diverges at the crossover (sin(u/2) -> 0) as expected.
Rename throughout: the KIEResult.infinite_parabola field, the printed table
header, get_row, docstrings, tests, README/DETAILS/AUTOQUIVER/TUTORIAL, and the
notebook (re-executed so outputs/headers refresh). "infinite" and "inverted"
are the same length, so fixed-width tables stay aligned.

Also credit Gino Occhialini for the Skodje-Truhlar tunnelling correction in the
README acknowledgements.

195 tests, 100% coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- DETAILS.md: add Wigner (1932), Bell (1959 + 1980 book) references
  alongside the existing Chem. Soc. Rev. review; fix QUIVER citation
  year (1989) and align the CLI help block with actual `pyquiver -h`
- notebook: cite Wigner/Bell in the tunnelling cell; clarify that only
  Bell is the infinite-parabola model
- test_batch: assert the Skodje-Truhlar batch column equals a direct
  KIE_Calculation.skodje_truhlar call (guards the energy-tuple mapping)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ekwan
ekwan merged commit 1781626 into master Jun 17, 2026
6 checks passed
@ekwan
ekwan deleted the refactor/package-and-api branch June 17, 2026 23:04
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.

1 participant