Set Pxx value to unsigned, limit to 1-99#2160
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the PV Uncertainty “Custom Px” input to prevent negative values and constrain the selectable P-value range, addressing out-of-bounds P-values in the Uncertainty tool (Fixes #1970).
Changes:
- Switches the “Custom Px”
wxNumericCtrlto an unsigned numeric mode. - Adds a 1–99 allowed range constraint to the P-value input.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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 ); | ||
| m_puser = new wxNumericCtrl( this, wxID_ANY, 90, wxNUMERIC_UNSIGNED ); |
There was a problem hiding this comment.
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.
| 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 ); |
There was a problem hiding this comment.
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.
Pull Request Template
Description
-Update PXX input on PV Uncertainty form to be unsigned, limited to 1-99
Fixes #1970
Corresponding branches and PRs:
develop everywhere else
Unit Test Impact:
[ new tests written? ]
NO
[ expected changes in unit tests or speed of tests? ]
None
[ expected changes in test_results files? ]
None
Checklist
Reminders- this section can be deleted
[Checking for PySAM Incompatible API Changes]
(https://github.com/NREL/SAM/wiki/PySAM-Incompatible-API-Changes-&-Regenerating-PySAM-Files).
[When do the PySAM files need to be regenerated?]
(https://github.com/NREL/SAM/wiki/PySAM-Incompatible-API-Changes-&-Regenerating-PySAM-Files#when-do-the-pysam-files-need-to-be-regenerated-via-export_config)