Skip to content

refactor(tilib): narrow ti_w fixture except → TilibParamError (#218) - #225

Merged
k-yoshimi merged 1 commit into
developfrom
feat/ti-w-except-narrowing-218
Jun 8, 2026
Merged

refactor(tilib): narrow ti_w fixture except → TilibParamError (#218)#225
k-yoshimi merged 1 commit into
developfrom
feat/ti-w-except-narrowing-218

Conversation

@k-yoshimi

@k-yoshimi k-yoshimi commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

Completes the ti_w portion of #218 that was deferred in PR #220 (ti_ar only). Mirrors PR #220's commits 23d5b889 + 6ca586f7 on python/tilib/tests/fixtures/ti_w_params.py:

  • Add from tilib.errors import TilibParamError import
  • Replace 4 except Exception: blocks with except TilibParamError: + updated pragma comment

Closes #218.

Why deferred (until now)

ti_w_params.py only existed on the #219 PR branch (`feat/fixture-parity-215`). #219 merged at `a12982dd` (2026-06-08), bringing the file onto develop. This follow-up was unblocked at that point.

PR #220 description noted this explicit deferral.

Pattern parity with PR #220

Same 4 narrowing sites, same import placement, same pragma comment string ("ierr=1 = invalid name or value; for UNREGISTERED_KEYS this is unknown-name and intentionally silenced"). ti_ar_params.py and ti_w_params.py now have identical exception-handling discipline.

ti_min_params.py has no try/except blocks (verified pre-#220) → #218 is fully closed.

Verified narrowing chain (re-confirmed during #220 review)

  • `ti/ti_api.h:27` `TI_ERR_INVALID = 1` ("invalid parameter name or value")
  • `ti/ti_param_registry.f90` returns ierr=1 for unknown name AND bad value
  • `python/tilib/errors.py:46-51` `_CODE_MAP[1] = TilibParamError`

Other exception classes — `TilibStateError` (ierr=2), `TilibRunError` (ierr=3), `TilibNotImplementedError` (ierr=4) — now propagate instead of being silenced.

Test plan

  • Local: `PYTHONPATH=python pytest python/tilib/tests/ --timeout=120 -q` → 43 passed, 3 skipped (no regression)
  • CI: python-tests.yml job passes
  • Bugbot COMPLETED

Pre-push gate

  • ✅ Local pytest: 43/3 (clean)
  • ✅ In-house code review: READY-TO-PUSH, no HIGH/MED
  • ✅ Codex independent review: READY-TO-PUSH, no HIGH/MED
  • ✅ REVIEW_OK marker placed for HEAD `4a986fe3`

🤖 Generated with Claude Code


Note

Low Risk
Test-fixture-only change with no production API or runtime logic modified; aligns error handling with an already-shipped ti_ar pattern.

Overview
In ti_w_params.py, the test fixture’s apply / _apply_array helpers now catch TilibParamError instead of bare Exception when calling ti.set_param, with the same pragma comment used in ti_ar_params.py.

Behavior for UNREGISTERED_KEYS is unchanged: invalid-name failures are still swallowed only for those keys. TilibStateError, TilibRunError, and other non-param errors are no longer accidentally suppressed.

Reviewed by Cursor Bugbot for commit 4a986fe. Bugbot is set up for automated code reviews on this repo. Configure here.

…ror (#218)

Completes the ti_w portion of #218 that was deferred in PR #220 (ti_ar
narrowing only). The ti_w fixture lived on the feat/fixture-parity-215
branch (PR #219, merged at a12982d) so this follow-up was unblocked
once #219 landed on develop.

Same pattern as PR #220's commits 23d5b88 + 6ca586f:
- Add `from tilib.errors import TilibParamError` import
- Replace 4 `except Exception:` blocks with `except TilibParamError:`
- All 4 sites: _apply_array dict branch, _apply_array list branch,
  apply SCALARS loop, apply MATRIX_ARRAYS loop

Narrowing chain verified during #220 review (ti_api.h:27 / errors.py:46-51):
- TI_ERR_INVALID = 1 → TilibParamError (unknown name OR invalid value)
- TI_ERR_NOT_INIT = 2, CALC_FAILED = 3, NOT_IMPL = 4 now propagate
  instead of being silenced for UNREGISTERED_KEYS keys.

ti_min_params.py has no try/except blocks (verified pre-#220) — no
further ti narrowing pending.

Closes #218

Local: 43 passed, 3 skipped (no regression).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@k-yoshimi

Copy link
Copy Markdown
Owner Author

@cursor review

ti_w narrowing follow-up to #220 (ti_ar). Mirrors PR #220 pattern exactly. Closes #218.

@cursor cursor Bot 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4a986fe. Configure here.

@k-yoshimi
k-yoshimi merged commit 67431cc into develop Jun 8, 2026
4 checks passed
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