Skip to content

More consistent error types#2058

Open
devmotion wants to merge 2 commits intomasterfrom
dmw/domainerror
Open

More consistent error types#2058
devmotion wants to merge 2 commits intomasterfrom
dmw/domainerror

Conversation

@devmotion
Copy link
Copy Markdown
Member

In a numerical estimation problem, I encountered numerical issues resulting in NaN parameters which then threw a DomainError when constructing a Normal distributions. So fine, so good - I could easily handle it by catching and handling the DomainError gracefully. Only later I noticed that similar numerical problems with Uniform will lead to an ArgumentError.

I think it's more convenient and more predictable for downstream users and packages if incorrect/infeasible parameter values always lead to DomainError.

devmotion and others added 2 commits April 22, 2026 23:42
Distribution constructors now throw a consistent error type based on the
nature of the violation:

* `DomainError(val, msg)` for numerically invalid parameter values. For
  relational checks across multiple parameters (e.g. `a < b` in Uniform,
  `lower ≤ upper` in Censored, `1 ≤ rank ≤ n` in OrderStatistic), the
  named tuple of related parameters is passed as `err.val` so downstream
  catchers can identify each value by name.
* `DimensionMismatch` for structural size checks in constructors — rows/cols
  of `M` vs `U`/`V`/`Σ`/`Ω` in `MatrixNormal`/`MatrixTDist`, and length of
  support vs probability vector in `DiscreteNonParametric`. This matches
  the idiom already used across the library for `suffstats`/`fit_mle`.
* `ArgumentError` is retained only for non-parameter, non-dimension issues
  (empty data in `fit_mle`, invalid enum in `_char_uplo`, etc.).

The `@check_args` macro is collapsed to two `DomainError`-based forms,
`(arg, cond, message)` and `(arg, cond)`, both requiring an offending
value. The previous no-value forms that fell back to `ArgumentError` have
been removed; the macro now errors at expansion time if a caller omits
the value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.63%. Comparing base (cc1a9fa) to head (8030aa0).

Files with missing lines Patch % Lines
src/utils.jl 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2058      +/-   ##
==========================================
- Coverage   86.65%   86.63%   -0.02%     
==========================================
  Files         148      148              
  Lines        8919     8914       -5     
==========================================
- Hits         7729     7723       -6     
- Misses       1190     1191       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@devmotion devmotion marked this pull request as ready for review April 23, 2026 07:38
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.

2 participants