Skip to content

This closes #2414, make a negative zero the same zero as any other - #2424

Merged
xuri merged 4 commits into
qax-os:masterfrom
PhilflowIO:2414-numeric-equality
Oct 8, 2026
Merged

xuri merged 4 commits into
qax-os:masterfrom
PhilflowIO:2414-numeric-equality

Conversation

@PhilflowIO

Copy link
Copy Markdown
Contributor

PR Details

Description

Normalises a negative zero in newNumberFormulaArg. Every number the engine produces passes through that constructor, so one place covers all paths.

Related Issue

Closes #2414

Depends on the MINVERSE fix in #MMMM. On its own this change breaks the existing expectation "MINVERSE(A1:B2)": "-0", because the 2x2 inverse is all zeros until that one lands.

Motivation and Context

A negative zero formats as "-0" and the equality operators compare the formatted text, so x = 0 was FALSE for a value that is zero. COUNTIF does not go through the comparison operators and saw the same thing, which is why this is fixed where the number is made and not where it is compared. The ordering operators compare the numbers and were never affected.

How Has This Been Tested

Cases added to TestCalcCellValue: a cell holding =(1=2)*-1 read back, compared with = and <>, used as an IF condition, and counted with COUNTIF. Ordering operators and ordinary zeros are covered as unchanged.

go test ./... passes on go1.26.8 linux/amd64. gofmt -s -l . is empty.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

This branch also contains the commit from #2420. Only the second commit belongs to this PR; after #2420 is merged this branch will be rebased so that the diff is the negative zero change alone.

PhilflowIO and others added 2 commits September 20, 2026 16:29
The cofactors of a 2x2 matrix are 1x1 determinants. Without a 1x1 case they fell into the Laplace expansion, which takes the minor of a 1x1 matrix, gets an empty matrix and sums over nothing, so every cofactor was 0 and MINVERSE of a 2x2 matrix was all zeros. MDETERM was not affected because `det` has its own 2x2 branch and never descends, which is why the existing tests did not catch it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: PhilflowIO <tech@philflow.io>
A negative zero formats as "-0" and the equality operators compare the formatted text, so `x = 0` was FALSE for a value that is zero. COUNTIF does not go through the comparison operators and saw the same thing, which is why this is fixed where the number is made and not where it is compared. The ordering operators compare the numbers and were never affected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: PhilflowIO <tech@philflow.io>
@xuri xuri added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 21, 2026
Comment thread calc_test.go Outdated

@xuri xuri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your pull request. I resolved code conflicts and made some changes based on your code branch.

@xuri xuri added this to v2.11.1 Oct 8, 2026
@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (efb5918) to head (cfdda36).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2424   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files          32       32           
  Lines       32524    32527    +3     
=======================================
+ Hits        32434    32437    +3     
  Misses         88       88           
  Partials        2        2           
Flag Coverage Δ
unittests 99.72% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@xuri
xuri merged commit 1cf5392 into qax-os:master Oct 8, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

Status: Bugfix

Development

Successfully merging this pull request may close these issues.

A negative zero is not equal to zero in = and <> and is not counted by COUNTIF

2 participants