Skip to content

Comments

[jaspQualityControl] Fix incorrect vertical axis titles on surface plots in Quality Cont...#433

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

[jaspQualityControl] Fix incorrect vertical axis titles on surface plots in Quality Cont...#433
sisyphus-jasp wants to merge 1 commit intojasp-stats:masterfrom
sisyphus-jasp:fix-sisyphus-1771909649

Conversation

@sisyphus-jasp
Copy link

Summary

Issue: jasp-stats/jasp-issues#4094

Fix: Incorrect vertical axis titles on surface plots

Root Cause

In .doeAnalysisPlotContourSurface() (R/doeAnalysis.R), the function correctly iterates over dependent variables with for (dep in dependent) and uses dep for plot titles. However, it incorrectly passed the full dependent vector (e.g., c("Y1", "Y2", "Y3")) to .doeContourSurfacePlotObject() instead of the loop variable dep.

When the plotting function received the vector c("Y1", "Y2", "Y3") and used it for zlab = dependent, R converted it to a string representation, resulting in incorrect axis labels showing only the first element "Y1" instead of the actual dependent variable being plotted (e.g., "Y2").

What Changed

Changed lines 1861 and 1863 in R/doeAnalysis.R:

  • Before: plot$plotObject <- function(){.doeContourSurfacePlotObject(result, options, dependent, variablePair, ...)}
  • After: plot$plotObject <- function(){.doeContourSurfacePlotObject(result, options, dep, variablePair, ...)}

This passes the current dependent variable from the loop iteration to the plot function, ensuring the correct z-axis label is displayed.

Testing

  • The baseline had 760 failures and 234 passes (pre-existing issues)
  • No existing tests for surface plot functionality were found in test-doeAnalysis.R
  • The fix is minimal and targeted - only changes the variable passed to the plot function

Implementation Plan

Plan: Fix incorrect vertical axis titles on surface plots

Root Cause

In .doeAnalysisPlotContourSurface (lines 1861 and 1863 of R/doeAnalysis.R):

  • The function iterates over dependent variables with for (dep in dependent) (line 1828)
  • It correctly uses dep for plot titles (line 1857)
  • BUT it incorrectly passes the full dependent vector (e.g., c("Y1", "Y2", "Y3")) to .doeContourSurfacePlotObject
  • In .doeContourSurfacePlotObject, line 1892 uses zlab = dependent to set the z-axis label
  • When passed a vector like c("Y1", "Y2", "Y3"), R converts it to string "c(\"Y1\", \"Y2\", \"Y3\")" for display

This is why the z-axis shows "Y1" (first element of the incorrectly passed vector) instead of the correct dependent variable (e.g., "Y2").

Proposed Changes

Change lines 1861 and 1863 in R/doeAnalysis.R:

  • Line 1861: Change dependent to dep
  • Line 1863: Change dependent to dep

This passes the current dependent variable from the loop iteration to the plot function.

Test Impact

  • Existing tests should still pass because this fixes incorrect behavior
  • The z-axis labels will now correctly show the dependent variable being plotted

Test Results

Test Run Result
Baseline (pre-fix) [ FAIL 760
Post-fix [ FAIL 760
Upstream CI ef34206 -- CI: failing

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.86%. Comparing base (3f4e354) to head (65193f1).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
R/doeAnalysis.R 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #433   +/-   ##
=======================================
  Coverage   80.86%   80.86%           
=======================================
  Files          17       17           
  Lines        8876     8876           
=======================================
  Hits         7178     7178           
  Misses       1698     1698           

☔ View full report in Codecov by Sentry.
📢 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.

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

3 participants