Skip to content

Proper DeprecationWarning for 'f' cone + tighten settings validation#199

Merged
bodono merged 1 commit into
masterfrom
fix/deprecation-and-validation
Apr 22, 2026
Merged

Proper DeprecationWarning for 'f' cone + tighten settings validation#199
bodono merged 1 commit into
masterfrom
fix/deprecation-and-validation

Conversation

@bodono

@bodono bodono commented Apr 22, 2026

Copy link
Copy Markdown
Owner

Summary

Two independent cleanups surfaced during the `scsobject.h` review:

1. `f` cone deprecation is now a real Python warning

The deprecation notice for the old `'f'` cone field was being printed via `scs_printf` to stdout. That meant users couldn't:

  • Filter it with `warnings.filterwarnings`
  • Capture it in logs or tests
  • Promote it to an error to catch remaining usage during a migration

Switched to `PyErr_WarnEx(PyExc_DeprecationWarning, ...)`, so it integrates with the standard `warnings` machinery and `pytest.warns`. If the warning has been promoted to an error (e.g. `warnings.filterwarnings("error")`), `SCS_init` cleans up and returns `-1` with the exception propagated.

2. Tighten settings validation

Previously the Python-side checks in `SCS_init` rejected only strictly negative values for several settings. Zero would slip through Python and get rejected later by SCS's own `validate()` (in `scs_source/src/scs.c`), surfacing as a generic "ScsWork allocation error" without naming the bad setting.

Tightened to match SCS's actual rules and corrected the messages:

  • `m`, `n`, `max_iters`, `acceleration_interval`, `rho_x`: `<= 0` now rejected (was `< 0`).
  • `alpha`: `<= 0 || >= 2` now rejected, message "alpha must be in (0, 2)" (was `< 0` with a misleading "must be positive").
  • `eps_abs`, `eps_rel`, `eps_infeas`: behavior unchanged, but message corrected from "positive" to "nonnegative" (zero is accepted).

Test plan

  • `pytest test/` — 327 passed, 66 skipped (up from 322; +5 new tests)
  • New tests:
    • `test_f_cone_deprecation_can_be_promoted_to_error` — verifies `filterwarnings("error")` converts the warning to a catchable exception.
    • `test_alpha_zero_raises`, `test_alpha_two_raises`, `test_rho_x_zero_raises`, `test_acceleration_interval_zero_raises` — verify each tightened bound raises `ValueError` at `init` with a specific message.
  • Existing `test_deprecated_f_cone_field` and `test_f_and_z_both_set_sum` updated to assert the DeprecationWarning fires via `pytest.warns`.
  • Existing `test_max_iters_zero_raises`, `test_scale_zero_raises` strengthened to match the specific error message and to trigger at `init` (no longer need the `.solve()` call that used to be where the error surfaced).

- The 'f' cone field deprecation notice was printed via scs_printf to
  stdout, so users couldn't filter, capture, or promote it to an error.
  Switched to PyErr_WarnEx(PyExc_DeprecationWarning, ...), which
  integrates with warnings.filterwarnings and pytest.warns.

- Tighten SCS_init parameter checks to match SCS's own validate() in
  scs_source/src/scs.c. Previously the Python checks rejected only
  strictly negative values for max_iters, alpha, rho_x,
  acceleration_interval, m, n — then SCS's own validate would reject
  zero later with a generic "ScsWork allocation error" message. Now
  users get a specific Python ValueError naming the offending setting:

  * m, n, max_iters, acceleration_interval, rho_x: tightened to `<= 0`
  * alpha: tightened to `<= 0 || >= 2` (message: "must be in (0, 2)")
  * eps_abs/rel/infeas: behavior unchanged (< 0 still rejected, zero
    still accepted), but messages corrected from "positive" to
    "nonnegative" to match the actual behavior.

- Tests added for the DeprecationWarning (both default and
  filterwarnings("error") paths) and for each tightened bound.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@bodono
bodono merged commit d3344a1 into master Apr 22, 2026
35 checks passed
@bodono
bodono deleted the fix/deprecation-and-validation branch April 22, 2026 20:07
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