Skip to content

Introduce PedAnovaBuilder for PedAnovaImportanceEvaluator - #262

Draft
Alnusjaponica wants to merge 2 commits into
mainfrom
feat/importance-builder
Draft

Alnusjaponica wants to merge 2 commits into
mainfrom
feat/importance-builder

Conversation

@Alnusjaponica

@Alnusjaponica Alnusjaponica commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Replace the positional constructor arguments of PedAnovaImportanceEvaluator with a builder following the API style of std::thread::Builder: settings are configured by chaining setter methods and the evaluator is created with build. Builder::new() is the only entry point, matching thread::Builder::new().

let evaluator = PedAnovaBuilder::new()
    .target_quantile(0.2)
    .region_quantile(0.9)
    .evaluate_on_local(false)
    .n_steps(100)
    .prior_weight(2.0)
    .min_n_trials_in_regime(3)
    .build()
    .unwrap();

Changes

  • Add PedAnovaBuilder (with Default) exposing chainable setters and build()
  • The builder exposes all evaluator settings: target_quantile, region_quantile, evaluate_on_local, n_steps, prior_weight, and min_n_trials_in_regime, which were previously fixed to their internal defaults
  • Remove the builder() entry point from PedAnovaImportanceEvaluator in favor of Builder::new() (no dual entry points)
  • new and Default are kept for backward compatibility and now delegate to the builder
  • The validation (0.0 < target_quantile < region_quantile <= 1.0) moved into build
  • Update the pyo3 binding to construct the evaluator through the builder

Verification

  • cargo test --all-features — 273 tests pass (including a new test_builder and doctests)
  • cargo test -p rustuna_storage --locked -- --ignored (from rustuna_pyo3) — 3 pass
  • cargo clippy --locked --workspace --lib --bins --tests --examples --all-features -- -D warnings — clean
  • cargo fmt --all -- --check — clean
  • Python: uv run --no-sync pytest tests/ — 701 pass (constructor, ValueError, and warning behavior verified through the new builder path)

Replace the positional constructor arguments of
PedAnovaImportanceEvaluator with a builder following the API style of
std::thread::Builder: settings are configured by chaining setter
methods and the evaluator is created with `build`.

The builder exposes all evaluator settings: target_quantile,
region_quantile, evaluate_on_local, n_steps, prior_weight and
min_n_trials_in_regime, which were previously fixed to their internal
defaults. `new` and `Default` are kept for backward compatibility and
delegate to the builder. The validation moved into `build`.
@Alnusjaponica
Alnusjaponica marked this pull request as draft September 28, 2026 16:18
PedAnovaImportanceEvaluator::builder() was redundant with
PedAnovaBuilder::new(). Keep Builder::new() as the only entry point,
matching std::thread::Builder.
@c-bata

c-bata commented Oct 5, 2026

Copy link
Copy Markdown
Member

@kAIto47802 Could you review this PR?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants