Skip to content

Conversation

@wahln
Copy link
Contributor

@wahln wahln commented Sep 3, 2025

Bug report #867 highlighted that, when pln.propStf.isoCenter contains multiple, differing isocenters, updates in the PlanWidget will parse the textbox value ("multiple isoCenters") incorrectly and assign NaN to pln.propStf.isoCenter.

This PR introduces:

  • A test to capture and reproduce this scenario
  • A check for the string value in case of multiple isocenters, avoiding overwriting of the field
  • An additional cleanup restoring octave compatibility of PlanWidget field changes

Fixes #867

@wahln wahln requested a review from Copilot September 3, 2025 15:44
@wahln wahln self-assigned this Sep 3, 2025
@wahln wahln added the bug label Sep 3, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the GUI would overwrite multiple isocenters with NaN values when updating plan parameters. The issue occurred when the PlanWidget tried to parse the "multiple isoCenter" string as a numeric value.

  • Adds test coverage for the multiple isocenter scenario
  • Implements a string check to prevent overwriting when "multiple isoCenter" is displayed
  • Improves Octave compatibility by storing object tag in a variable

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/gui/test_gui_PlanWidget.m Adds comprehensive test to verify multiple isocenter handling during GUI updates
matRad/gui/widgets/matRad_PlanWidget.m Implements fix to preserve multiple isocenters and improves Octave compatibility

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Sep 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.17%. Comparing base (c67b2a9) to head (cbb274c).
⚠️ Report is 5 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #868      +/-   ##
==========================================
+ Coverage   53.07%   53.17%   +0.10%     
==========================================
  Files         307      307              
  Lines       19819    19822       +3     
==========================================
+ Hits        10519    10541      +22     
+ Misses       9300     9281      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link

github-actions bot commented Sep 3, 2025

Test Results

  3 files  ±0    3 suites  ±0   42m 42s ⏱️ +19s
289 tests +1  289 ✅ +1  0 💤 ±0  0 ❌ ±0 
963 runs  +3  958 ✅ +3  5 💤 ±0  0 ❌ ±0 

Results for commit cbb274c. ± Comparison against base commit c67b2a9.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 3, 2025

Code Coverage

Package Line Rate Health
coverage Package 1 53%
Summary 53% (10534 / 19822)

@wahln wahln merged commit 3b0e262 into dev Sep 3, 2025
15 checks passed
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.

2 participants