Skip to content

Validate binary calibration target values - #3967

Open
maziyarpanahi wants to merge 18 commits into
masterfrom
fix/issue-3568-calibration-targets
Open

maziyarpanahi wants to merge 18 commits into
masterfrom
fix/issue-3568-calibration-targets

Conversation

@maziyarpanahi

@maziyarpanahi maziyarpanahi commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Pull Request

Description

Accept calibration targets only as booleans or numeric zero/one. Non-empty strings such as "false" must not silently become positive examples and distort calibration.

This follows the report and immutable reproduction shared by Belal Embaby. The fix was reproduced against current upstream and implemented in existing tracked source/test files; the contributor's fork was not modified.

Type of Change

  • Bug fix
  • Test addition/improvement

Changes Made

  • Validate target, is_true and matched aliases with a fixed value-free error.
  • Preserve alias precedence, the absent-target default and all accepted boolean/integer/float zero/one inputs.
  • Extend the existing calibration tests for strings, containers, null, nonbinary and nonfinite numbers, fitting-path rejection and unchanged defaults; update docstring/changelog.

Testing

The published candidate is 1e797b14521d4e989a8c1221d1053666b553989c. It retains the issue contribution and normally integrates frozen SDK prerequisite 5e635dd707ebd74c2d0806b0cb6415b53ab4d7b0 from #3971. The issue-specific scope is unchanged; shared SDK/CI/build-budget changes belong to that prerequisite.

  • make format, make lint, make format-check, make type-check and git diff --check: passed, without unrelated formatting changes.
  • The repository's configured type check covers four modules, not the entire codebase.
  • Focused regression/public-API tests: 125 passed.
  • Shared wheel/import budget and CI-wiring tests: 41 passed.
  • Repository-policy and declared development-profile dependency audit: passed. This audit does not clear separately reviewed optional dependency profiles.
  • High-severity/medium-confidence Bandit gate: passed, zero high-severity findings, no files skipped. Lower-severity results are not represented as zero.

Post-integration full offline pytest passed on this exact head: 23,207 passed, 222 skipped, 24 warnings; zero failures/errors, in 448.15s. Local runtime: Python 3.13.11, with a private environment and unique OS temporary fixture directory. The full run captured exit 0 and the source remained clean and unchanged. Skips and warnings are not counted as executed passes. Earlier full receipts refer to older heads and are superseded only for this candidate.

SDK #3971 must still land and this exact pushed head must pass its own full local and hosted checks before merge. Hosted checks remain pending; this is not merge or release clearance. No tests or assertions were removed or weakened, and no skipped check or dependency waiver is introduced.

Documentation

  • Updated the relevant public docstring and CHANGELOG.md.

Code Quality

  • Canonical formatting/lint/type gates passed.
  • Performed a separate diff review and checked accepted-input/negative controls.
  • No unrelated formatting changes or new tracked files.

Dependencies

Checklist

  • Read the contributing guide, Code of Conduct and maintainer guide.
  • One focused issue implementation commit is retained; shared prerequisite integration is recorded separately. The contributor's original fork remains unchanged.

Related Issues

Closes #3568

Screenshots/Examples

Not applicable; backend/evaluation contract fix with synthetic unit coverage.

@maziyarpanahi maziyarpanahi added this to the v3.0 milestone Oct 3, 2026
@maziyarpanahi maziyarpanahi added bug Something isn't working roadmap-v3 OpenMed V3 roadmap backlog labels Oct 3, 2026
@maziyarpanahi maziyarpanahi self-assigned this Oct 3, 2026
@maziyarpanahi maziyarpanahi added bug Something isn't working roadmap-v3 OpenMed V3 roadmap backlog labels Oct 3, 2026
@maziyarpanahi
maziyarpanahi marked this pull request as ready for review October 3, 2026 12:05

Copy link
Copy Markdown
Owner Author

Thank you for the focused contribution for #3568. Its implementation and regression coverage already landed on master through #3650, and the issue is closed. I verified the release merge is present and the corresponding regressions still pass on current master.

This PR is held as superseded; the resolved issue provides no remaining implementation scope for another merge. I am retaining the original branch and discussion. Any additional behavior should be scoped to a new maintainer-backed issue before further changes or merge.

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

Labels

bug Something isn't working roadmap-v3 OpenMed V3 roadmap backlog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Reject ambiguous calibration target values

2 participants