Skip to content

Set Pxx value to unsigned, limit to 1-99 #2160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mjprilliman wants to merge 2 commits into
base: develop
Choose a base branch
from
+6 −1
Open

Set Pxx value to unsigned, limit to 1-99 #2160

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e54e08
Set Pxx value to unsigned, limit to 1-99
mjprilliman Apr 1, 2026
94f51cc
Update bounds on Pxx when loading existing cases
mjprilliman Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/pvuncertainty.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,12 @@ PVUncertaintyForm::PVUncertaintyForm( wxWindow *parent, Case *cc )

wxStaticBoxSizer *sizer_changePvalue = new wxStaticBoxSizer( wxHORIZONTAL, this, "Update P value" );
label = new wxStaticText( this, wxID_ANY, "Custom Px:" );
m_puser = new wxNumericCtrl( this, wxID_ANY, 90, wxNUMERIC_REAL );
int pValue = static_cast<int>(m_data.pValue);
if (pValue < 1) pValue = 1;
else if (pValue > 99) pValue = 99;
m_data.pValue = pValue;
m_puser = new wxNumericCtrl( this, wxID_ANY, 90, wxNUMERIC_UNSIGNED );
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The UI range/unsigned restriction here only affects what the user can type going forward; m_data.pValue can still be out of bounds when loading an existing case file (see PVUncertaintyData::Read() and UpdateFromSimInfo() setting the control from m_data.pValue). Consider clamping/normalizing the value (and persisting it back to m_data.pValue) when reading/applying it so legacy projects with pValue <=0 or >=100 don’t continue running with invalid internal state even if the control display gets constrained.

Suggested change
m_puser = new wxNumericCtrl( this, wxID_ANY, 90, wxNUMERIC_UNSIGNED );
int pValue = static_cast<int>(m_data.pValue);
if (pValue < 1) pValue = 1;
else if (pValue > 99) pValue = 99;
m_data.pValue = pValue;
m_puser = new wxNumericCtrl( this, wxID_ANY, pValue, wxNUMERIC_UNSIGNED );

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

@mjprilliman mjprilliman Apr 1, 2026

Choose a reason for hiding this comment

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

@sjanzou does that look more correct?

Copy link
Copy Markdown
Collaborator

@sjanzou sjanzou Apr 2, 2026

Choose a reason for hiding this comment

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

Yes, between the explicit pValue check and the m_puser->SetRange the P value should be limited to 1<=P<=99. if you test and that is not the case, you will need to override the OnChange event for m_puser.

m_puser->SetRange(1, 99);
sizer_changePvalue->AddSpacer( 20 );
sizer_changePvalue->Add( label , 0, wxLEFT|wxRIGHT|wxALIGN_CENTER_VERTICAL, 0 );
sizer_changePvalue->Add( m_puser, 0, wxALL|wxALIGN_CENTER_VERTICAL, 3 );
Expand Down
Loading