Skip to content

[jaspFrequencies] Fix incorrect display of test value in Binomial Test footnote. The...#300

Open
sisyphus-jasp wants to merge 1 commit into
jasp-stats:masterfrom
sisyphus-jasp:fix-sisyphus-1771952298
Open

[jaspFrequencies] Fix incorrect display of test value in Binomial Test footnote. The...#300
sisyphus-jasp wants to merge 1 commit into
jasp-stats:masterfrom
sisyphus-jasp:fix-sisyphus-1771952298

Conversation

@sisyphus-jasp

Copy link
Copy Markdown

Summary

Fixes: https://github.com/jasp-stats/INTERNAL-jasp/issues/3110

Issue #3110: Fix incorrect display of test value in Binomial Test footnote

What was wrong (root cause)

The Binomial Test footnote was displaying the raw formula string (e.g., "rnorm(1)") instead of the actual parsed numeric value. This happened because the code used options$testValueUnparsed which contains the unparsed formula, instead of options$testValue which contains the actual numeric value that was parsed and used in calculations.

What was changed

Changed the footnote generation in both binomialtest.R and binomialtestbayesian.R to use options$testValue instead of options$testValueUnparsed:

  • R/binomialtest.R (lines 199, 201, 203): Changed from options$testValueUnparsed to options$testValue
  • R/binomialtestbayesian.R (lines 122, 124, 126): Changed from options$testValueUnparsed to options$testValue

Why this fix works

The .parseAndStoreFormulaOptions() function (called at line 28 in binomialtest.R and line 22 in binomialtestbayesian.R) parses formula fields and stores:

  • The parsed/evaluated value in options$testValue
  • The raw formula string in options$testValueUnparsed

The footnote should display the actual value being used in the test (the parsed value), not the formula string the user entered.

Test impact

  • Tests pass with the same results as baseline: [ FAIL 14 | WARN 9 | SKIP 3 | PASS 96 ]
  • No new failures introduced
  • All existing failures are pre-existing plot-related snapshot tests unrelated to this fix

Caveats for reviewer

  • The same pattern exists for priorAUnparsed and priorBUnparsed in the Bayesian binomial test (line 128), but this was not part of the reported issue. A separate issue could be filed for that.

Implementation Plan

Issue #3110: Fix incorrect display of test value in Binomial Test footnote

Root Cause

The footnote in the Binomial Test output displays the raw formula string (e.g., "rnorm(1)") instead of the actual parsed test value. This happens because the code uses options$testValueUnparsed which contains the unparsed formula string, when it should use options$testValue which contains the actual parsed numeric value.

Affected Files

  1. R/binomialtest.R - Lines 199, 201, 203
  2. R/binomialtestbayesian.R - Lines 122, 124, 126

Proposed Changes

In both files, replace options$testValueUnparsed with options$testValue in the footnote generation:

  • Line 199: Change from options$testValueUnparsed to options$testValue
  • Line 201: Change from options$testValueUnparsed to options$testValue
  • Line 203: Change from options$testValueUnparsed to options$testValue

(The same change in binomialtestbayesian.R at lines 122, 124, 126)

Expected Test Impact

The existing tests should still pass because:

  • The footnote text will change from showing the formula string to showing the numeric value
  • The core functionality (test calculation) is unchanged
  • Tests that verify table structure will pass, but tests that verify exact footnote text may need updating

The tests affected will likely be snapshot tests that capture the footnote text.

Test Results

Test Run Result
Baseline (pre-fix) [ FAIL 14 | WARN 9 | SKIP 3 | PASS 96 ]
Post-fix [ FAIL 14 | WARN 9 | SKIP 3 | PASS 96 ]
Upstream CI 6a970c4 -- CI: failing

@FBartos

FBartos commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

I'm actually not sure whether we want this fix or somehow differentiating between

  1. r functions to be evaluated: runif(1) -> 0.358
  2. fractions to be reported as original values 1/3 -> 1/3

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.

5 participants