Skip to content

Fix nanmax handling of empty and logical inputs - #516

Merged
pr0m1th3as merged 1 commit into
gnu-octave:mainfrom
sahilphad07-sudo:fix-nanmax-empty-logical
Oct 6, 2026
Merged

pr0m1th3as merged 1 commit into
gnu-octave:mainfrom
sahilphad07-sudo:fix-nanmax-empty-logical

Conversation

@sahilphad07-sudo

Copy link
Copy Markdown
Contributor

nanmax did not correctly handle empty and logical inputs. Empty inputs could return NaN instead of preserving MATLAB-compatible empty outputs, while logical inputs caused an invalid conversion error because the implementation temporarily replaced values with -Inf and NaN.

This change updates nanmax to:

  • Preserve the correct empty-array output shapes.
  • Handle logical inputs using the underlying max behavior without converting logical values to NaN.
  • Preserve existing behavior for numeric and other supported inputs.

Before

octave:1> nanmax ([])
ans = NaN

octave:2> nanmax (logical ([0 1 0]))
error: invalid conversion from NaN to logical

After

octave:1> nanmax ([])
ans = [](0x0)

octave:2> nanmax (logical ([0 1 0]))
ans = 1

MATLAB Behavior

>> nanmax([])
ans =

     []

>> nanmax(logical([0 1 0]))
ans =

  logical

   1

Regression Tests Added

  • Empty input arrays and empty dimensions.
  • Empty arrays with explicit dimensions.
  • Empty arrays with 'all' and vecdim.
  • Logical vector and matrix inputs.
  • Logical inputs with explicit dimensions and 'all'.

Test Results

  • Existing nanmax tests passes.
  • Added regression tests pass.

@pr0m1th3as
pr0m1th3as merged commit 3780226 into gnu-octave:main Oct 6, 2026
8 checks passed
pr0m1th3as added a commit that referenced this pull request Oct 6, 2026
  * inst/Descriptive_Statistics/nanmax.m: Remove the logical branch;
  restore NaN only where X holds one.  Add tests.

  * inst/Descriptive_Statistics/nanmin.m: Likewise.
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