Skip to content

Throw a clear error for degenerate-length data vectors (fixes #121)#206

Open
JohnCobbler wants to merge 1 commit into
JuliaStats:masterfrom
JohnCobbler:fix/121-quantile-degenerate-range
Open

Throw a clear error for degenerate-length data vectors (fixes #121)#206
JohnCobbler wants to merge 1 commit into
JuliaStats:masterfrom
JohnCobbler:fix/121-quantile-degenerate-range

Conversation

@JohnCobbler

Copy link
Copy Markdown

Give quantile! a clear ArgumentError instead of an internal AssertionError
for a data vector that reports length 0 while not being empty, fixing #121.

An integer range whose length overflows (e.g. typemin(Int):typemax(Int)) has
length == 0 but isempty == false. Such input passes the isempty guard in
_quantilesort! and then trips @assert n > 0 inside _quantile, whose comment
claims the case "should never happen here".

This adds a length(v) == 0 guard in _quantilesort! (covering both quantile!
entry points before any _quantile call) that throws an ArgumentError pointing
the user at the overflow and suggesting they collect to a concrete vector first.
The now-redundant @assert in _quantile becomes a defensive ArgumentError,
which also keeps the check under --check-bounds=no where @assert may be elided.

r = typemin(Int):typemax(Int)
quantile!([0], r, [1])        # ArgumentError, was AssertionError
quantile(r, 0.5; sorted=true) # ArgumentError, was BoundsError

The default quantile(r, 0.5) path is unchanged: it materializes the range via
Base.copymutable first, which errors before reaching this code; that is a
separate, appropriate failure for a range claiming 2^64 elements.

Tests cover both overflow paths and confirm the existing empty-vector behavior
(ArgumentError("empty data vector")) still fires.

…ats#121)

A range whose length overflows (e.g. typemin(Int):typemax(Int)) reports
length 0 while not being empty, so it passed the isempty guard in
_quantilesort! and then tripped an internal '@Assert n > 0' in _quantile.
Add a length-0 guard that throws an informative ArgumentError, and turn the
now-redundant assertion into a defensive ArgumentError so the check also holds
under --check-bounds=no.

Co-authored-by: Claude <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.44%. Comparing base (e14c8f8) to head (06b7ad1).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #206   +/-   ##
=======================================
  Coverage   96.43%   96.44%           
=======================================
  Files           2        2           
  Lines         449      450    +1     
=======================================
+ Hits          433      434    +1     
  Misses         16       16           

☔ View full report in Codecov by Harness.
📢 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.

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