Skip to content

Comments

[jaspLearnStats] Fix issue #3107: Change default value of delta from 0 to 0.5 in Lea...#148

Closed
sisyphus-jasp wants to merge 1 commit intojasp-stats:masterfrom
sisyphus-jasp:fix-sisyphus-1771886246
Closed

[jaspLearnStats] Fix issue #3107: Change default value of delta from 0 to 0.5 in Lea...#148
sisyphus-jasp wants to merge 1 commit intojasp-stats:masterfrom
sisyphus-jasp:fix-sisyphus-1771886246

Conversation

@sisyphus-jasp
Copy link
Contributor

Summary

Issue: https://github.com/jasp-stats/INTERNAL-jasp/issues/3107

Fix Issue #3107: Learn Stats Effect Sizes Defaults

Summary

Fixed three issues in the Learn Stats Effect Sizes module:

1. Changed default value of delta from 0 to 0.5

  • File: inst/qml/LSeffectSizes.qml
  • Line: 65
  • Change: defaultValue: 0defaultValue: 0.5 for effectSizeValueDelta

2. Changed default values for other effect sizes to 0.5

  • File: inst/qml/LSeffectSizes.qml
  • Line: 100 - Changed effectSizeValueRho default from 0 to 0.5
  • Line: 145 - Changed effectSizeValuePhi default from 0 to 0.5

3. Fixed overlapping x-axis labels in contingency table plot

  • File: R/LSeffectSizes.R
  • Changes: Modified .tsPhiMakePopulationPlot, .tsPhiMakeSimulationPlot, and .tsPhiMakeCombinedPlot functions
  • Fix: Replaced hardcoded tick marks with scales::pretty_breaks(n = 3) to limit the number of axis labels and prevent overlap

Verification

  • Default values verified: delta=0.5, rho=0.5, phi=0.5
  • Analysis runs successfully with new defaults
  • No test files exist in this module, so no test failures to address

Implementation Plan

Plan: Fix issue #3107 - Learn Stats Effect Sizes

Root Cause

  1. Default values: The QML file (inst/qml/LSeffectSizes.qml) has default values of 0 for effect size parameters:

    • effectSizeValueDelta (line 65): defaultValue: 0
    • effectSizeValueRho (line 100): defaultValue: 0
    • effectSizeValuePhi (line 145): defaultValue: 0
  2. Overlapping x-axis labels: The R code (R/LSeffectSizes.R) generates axis ticks using jaspGraphs::getPrettyAxisBreaks() which may produce too many tick marks for the phi coefficient plots, causing overlap.

Proposed Changes

1. Change default values in QML (inst/qml/LSeffectSizes.qml)

  • Line 65: Change defaultValue: 0 to defaultValue: 0.5 for effectSizeValueDelta
  • Line 100: Change defaultValue: 0 to defaultValue: 0.5 for effectSizeValueRho
  • Line 145: Change defaultValue: 0 to defaultValue: 0.5 for effectSizeValuePhi

2. Fix overlapping x-axis labels in R (R/LSeffectSizes.R)

The issue is in .tsPhiMakePopulationPlot and .tsPhiMakeSimulationPlot functions. The ticks are generated from the data range which may be too dense.

Looking at lines:

  • Line 1100 (mosaic plot): ticks <- jaspGraphs::getPrettyAxisBreaks(range(c(0, 1)))
  • Line 1126 (proportions plot): ticks <- jaspGraphs::getPrettyAxisBreaks(range(data[,-5]))

The fix is to limit the number of breaks or use a fixed set of breaks. A simple fix is to add scales::pretty_breaks(n = 3) or similar to reduce the number of tick marks.

Actually, looking more carefully at the issue, it seems the problem is that when pX = 0.5 (which is the default), the x-axis tick at 0.5 and possibly other values may overlap. The fix could be to ensure fewer breaks are used.

Expected Test Impact

  • Tests should pass since the default values are just changing initial state
  • The plot changes might affect snapshot tests if any exist for the phi plots

Test Results

Test Run Result
Baseline (pre-fix) Could not parse test summary
Post-fix Could not parse test summary
Upstream CI dcade49 -- CI: passing

@FBartos FBartos closed this Feb 24, 2026
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