Skip to content

Python api update#6

Merged
mlund merged 8 commits into
masterfrom
python-api-docstrings
Jun 17, 2026
Merged

Python api update#6
mlund merged 8 commits into
masterfrom
python-api-docstrings

Conversation

@mlund

@mlund mlund commented Jun 16, 2026

Copy link
Copy Markdown
Owner

This pull request substantially improves the Python API documentation, fitting workflow, and code clarity for the pripps project, particularly in the README and example scripts. The changes clarify the usage of fit knobs, introduce and document the new built-in fitting interface, and update examples to use the streamlined API. Several scripts are refactored to remove redundant manual fitting code in favor of the new convenience methods, and minor code and documentation cleanups are made throughout.

Key changes:

Documentation and API Usage Improvements

  • The README.md is extensively rewritten to clarify installation instructions, explain the new built-in fitting interface (model.fit()), and provide updated code examples. It now emphasizes that fit knobs (volume_scale, contrast_density) are applied at the intensity calculation or fit stage, not at model construction, and documents trajectory averaging and model interface consistency. [1] [2] [3]
  • The documentation of model parameters in the README is updated to clarify the distinction between builder arguments and fit-time knobs, improving user understanding of the API.

Example Scripts Refactoring

  • pripps-py/examples/fit_lysozyme.py is refactored to demonstrate both the new built-in fitting method (model.fit(...)) and manual fitting with scipy.optimize.minimize, with clearer output and improved comments. The grid search is removed in favor of a simpler, more reproducible workflow. [1] [2]
  • pripps-py/examples/batch_single_bead_fit_sasbdb.py is updated to use the built-in model.fit method instead of manual optimization, and the hydration model is set to "grid" for improved consistency with documentation. Redundant code for manual scaling and optimization is removed. [1] [2] [3] [4]

Minor Code and Documentation Cleanups

  • Minor variable renames and formatting improvements are made in assets/stovgaard2010.py and across several Jupyter notebooks and scripts for better readability and PEP8 compliance. [1] [2] [3] [4] [5]
  • Plotting scripts are reformatted for clarity, improving code readability and maintainability. [1] [2] [3]

mlund added 2 commits June 16, 2026 20:02
pyo3 already exposed the per-method `///` docs as Python docstrings, but
the `pripps` module and the `Pripps` / `MultipoleModel` classes had none,
so `help(pripps)` and `help(pripps.Pripps)` were empty. Add `///` doc
comments on the `#[pymodule]` fn and each `#[pyclass]` struct; pyo3
surfaces these as `__doc__`.

Also declare numpy as a runtime dependency in pyproject.toml: every
array returned by the extension (the intensity() dict, q_values) is
created through rust-numpy, which needs numpy's C-API at call time.
Copilot AI review requested due to automatic review settings June 16, 2026 18:18
@mlund mlund added the enhancement New feature or request label Jun 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the Python-facing packaging and documentation for the pripps bindings, making it clearer how to install from source and providing richer Python docstrings for the exposed module and classes.

Changes:

  • Document uv pip install from the GitHub repo (with #subdirectory=pripps-py) and clarify prerequisites.
  • Add Python docstrings to the PyO3-exposed Pripps, MultipoleModel, and the top-level pripps module.
  • Declare NumPy as a runtime dependency for the Python package.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
README.md Adds an installation path via uv directly from GitHub and clarifies editable install for assets/examples.
pripps-py/src/lib.rs Adds Python docstrings for the module and primary exposed classes.
pripps-py/pyproject.toml Adds NumPy as an explicit runtime dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mlund added 4 commits June 16, 2026 20:35
These two classes arrived from master (PR #5) after the original
docstring commit, so they lacked the class-level `__doc__` that Pripps
and MultipoleModel already carry. Mirror the same description.
Add `#![deny(missing_docs)]` so every public item must be documented,
and document the public surface (crate-level summary, Intensity /
Contributions / IntensityScheme fields, solvent config enums/fields,
error variant fields, etc.).

Reduce the public API to what consumers (pyo3 bindings, benches, the CLI
binary) actually use: demote the atomkind and formfactor re-exports to
`pub(crate)`, make `fit` a private module, and drop the unused
`Pripps::formfactors` accessor and `FormFactor::in_qrange`. Gate the
test-only helpers `AtomKind::new` and `FormFactorMap::is_empty` behind
`#[cfg(test)]`.

The narrowing exposed `FitResult.chi2` as never read; surface it by
logging the fitted parameters and χ²/N from the CLI fit path.
Make the pyo3 bindings idiomatic and expose more of the core:

- intensity() now returns a typed `Intensity` (numpy `.q`/`.total`/
  `.atoms`/`.excluded`/`.hydration`; contributions are None when absent)
  instead of a dict with conditional keys.
- Model selection via `pripps.Excluded`/`Hydration`/`Scheme` enums,
  accepted interchangeably with lowercase strings.
- Drop `volume_scale`/`contrast_density` from the model builders; they
  are fit knobs applied at `intensity(volume_scale, contrast_density)`.
- Add `model.fit(q, intensity, sigma=None) -> FitResult` (built-in fit)
  on all three cached models; manual scipy fitting still works on the
  typed result.
- Add one-shot `Pripps.intensity(structure_file, *, model=..., xtc=...,
  stride=..., weights=..., box_sides=...)` exposing XTC trajectory
  averaging.

Core: re-export `fit::{ExpData, FitResult, fit_parameters}` and add
`ExpData::from_arrays` so the bindings can fit from numpy arrays.

Examples/tests/README updated to the new API (fit_lysozyme demonstrates
both built-in and scipy fitting with a neutral start); all Python files
and notebooks run through `ruff format`/`ruff check`.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

Comment thread README.md Outdated
Comment on lines +59 to +61
| `.atoms` | Protein-only (vacuum) contribution (`None` if no solvent) |
| `.excluded` | Excluded-volume contribution (`None` if no solvent) |
| `.hydration` | Hydration-shell contribution (`None` if no solvent) |
Comment thread pripps-py/src/lib.rs
Comment on lines +423 to +427
excluded_spacing = 1.0,
bulk_electron_density = 0.334,
volume_scale = 1.0,
contrast_density = 0.0,
))]
Comment thread pripps-py/src/lib.rs Outdated
Comment on lines +512 to +515
/// Compute I(q) for the given fit knobs, returning an
/// `Intensity` (numpy `.q` / `.total` / `.atoms` / ...).
#[pyo3(signature = (volume_scale = 1.0, contrast_density = 0.0))]
fn intensity(&self, volume_scale: f64, contrast_density: f64) -> PyIntensity {
Comment thread pripps-py/src/lib.rs Outdated
Comment on lines +140 to +149
/// Scattering vector magnitudes q (Å⁻¹).
#[getter]
fn q<'py>(&self, py: Python<'py>) -> Bound<'py, PyArray1<f64>> {
self.inner.q.clone().into_pyarray(py)
}

/// Total intensity I(q).
#[getter]
fn total<'py>(&self, py: Python<'py>) -> Bound<'py, PyArray1<f64>> {
self.inner.total.clone().into_pyarray(py)
Comment thread src/lib.rs Outdated
Comment on lines +59 to +63
" excluded=\"per-kind\",\n",
" volume_scale=1.0, # fit knob c₁ (scales excluded volume)\n",
" hydration=\"sasa\", # FoXS-style SASA-weighted hydration shell\n",
" probe=1.8, # SASA probe radius (Å)\n",
" contrast_density=0.03, # fit knob c₂ (scales hydration)\n",
" volume_scale=1.0, # fit knob c₁ (scales excluded volume)\n",
" hydration=\"sasa\", # FoXS-style SASA-weighted hydration shell\n",
" probe=1.8, # SASA probe radius (Å)\n",
" contrast_density=0.03, # fit knob c₂ (scales hydration)\n",
Comment on lines +169 to +173
" excluded=\"fraser\", # FoXS-style excluded volume (16π Fraser)\n",
" volume_scale=1.0, # fit knob c₁ (scales excluded volume)\n",
" hydration=\"sasa\", # FoXS-style SASA-weighted hydration shell\n",
" probe=1.8, # SASA probe radius (Å)\n",
" contrast_density=0.03, # fit knob c₂ (scales hydration)\n",
" volume_scale=1.0, # fit knob c₁ (scales excluded volume)\n",
" hydration=\"sasa\", # FoXS-style SASA-weighted hydration shell\n",
" probe=1.8, # SASA probe radius (Å)\n",
" contrast_density=0.03, # fit knob c₂ (scales hydration)\n",
mlund added 2 commits June 17, 2026 11:34
- PyIntensity now builds its NumPy arrays once and shares them on every
  attribute access (cheap reference clone) instead of cloning per access.
- Default `contrast_density` to 0.03 (matching the docs/CLI) on both the
  cached models' `intensity()` and the one-shot `Pripps.intensity`, so
  hydration isn't silently disabled when omitted.
- README: clarify that cached models always populate
  `.atoms`/`.excluded`/`.hydration` (zero when disabled); `None` only for
  trajectory averages.
- Remove the stale `Pripps::formfactors` reference in src/lib.rs.
- testing_single-bead_approximation.ipynb: drop the removed builder fit
  knobs and switch dict access to the typed Intensity attributes.
@mlund
mlund merged commit ce30253 into master Jun 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants