Skip to content

fix(autoreport): treat non-finite decimal values as not_provided DEV-1797#340

Merged
noliveleger merged 1 commit intomainfrom
dev-1797-treat-inf-and-nan-as-none
Mar 4, 2026
Merged

fix(autoreport): treat non-finite decimal values as not_provided DEV-1797#340
noliveleger merged 1 commit intomainfrom
dev-1797-treat-inf-and-nan-as-none

Conversation

@noliveleger
Copy link
Contributor

What was done

  • NumField.parse_values now raises a ValueError when a decimal field yields a non-finite float (Inf, -Inf, nan)
  • The existing ValueError handling in _calculate_stats already treats these as blank responses, incrementing not_provided without affecting provided or total_count

Why

Non-finite float values (Inf, -Inf, nan) can appear in submissions (e.g. from buggy clients or division-by-zero in calculated fields). Previously these were passed through silently, corrupting statistics (mean, median, etc.) and inflating provided/total_count.

Testing

  • Added test_stats_with_non_finite_float_value in tests/test_autoreport.py covering Inf, -Inf, and nan values for a decimal field
  • Verified that not_provided increments correctly and that provided/total_count remain accurate

@noliveleger noliveleger self-assigned this Mar 3, 2026
@coveralls
Copy link

Coverage Status

coverage: 86.852% (+0.05%) from 86.804%
when pulling d61cb23 on dev-1797-treat-inf-and-nan-as-none
into 085ae8e on main.

Copy link
Contributor

@rgraber rgraber left a comment

Choose a reason for hiding this comment

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

Request for 1 additional test

for i, stat in enumerate(stats):
assert stat == expected[i]

def test_stats_with_non_finite_float_value(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a test for when every response is infinite?

Copy link
Contributor

@rgraber rgraber left a comment

Choose a reason for hiding this comment

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

Approving for time's sake. Hopefully we can add tests when we're not in a rush for release

@noliveleger noliveleger merged commit e434285 into main Mar 4, 2026
1 check passed
@noliveleger noliveleger deleted the dev-1797-treat-inf-and-nan-as-none branch March 4, 2026 20:04
noliveleger added a commit to kobotoolbox/kpi that referenced this pull request Mar 4, 2026
…onses DEV-1797 (#6779)

### 📣 Summary

Fixed an issue where the reports page would fail to load when a decimal
field contained a non-finite value (e.g. `Infinity` or `NaN`) in a
submission.

Also fixes the statistics computation so that submissions containing
non-finite decimal values are treated as **not provided**, rather than
corrupting counts and aggregates.

### 📖 Description

When a submission contains `Infinity` or `NaN` for a decimal field, two
problems occur:

1. **Invalid JSON response (this PR):** The backend statistics
computation (mean, median, stdev, mode) can produce non-finite float
values. These are not valid JSON, causing `JSON.parse` in the browser to
fail and the reports page to display an error. Fixed by introducing a
`SanitizedJSONRenderer` that recursively replaces any non-finite float
(`NaN`, `Infinity`, `-Infinity`) with `null` before serializing the HTTP
response.

2. **Incorrect statistics
([formpack#340](kobotoolbox/formpack#340
`NumField.parse_values()` was silently passing non-finite floats
through, causing them to be counted as valid provided responses and
corrupting `provided`, `total_count`, and aggregate statistics (mean,
median, etc.). Fixed upstream by raising a `ValueError` for non-finite
floats — the existing `_calculate_stats` error handler already treats
these as `not_provided`. This PR bumps the formpack dependency to
include that fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants