Changed the two controls for saving the definitions (checkbox and sav…

[Emmanuel-Afrifa]

Fixes #10188
@lilyclements @rdstern @berylwaswa
This PR is ready for review. I was initially having an issue, but the AI's initial review gave me a fix to the problem. Thank you.

Before asking for review

Please confirm that you have:

Developer Testing Checklist

• Runs without errors
• OK disabled when dialog is incomplete or invalid
• OK enabled only when required inputs are valid
• Reset returns dialog to its default/sensible state
• Invalid inputs are handled cleanly (e.g. negative, too-large, empty, impossible combos)
• Running twice with different settings behaves consistently (e.g., open → run → close → reopen → change options checked → run again)
• All AI/bot comments addressed (fixed, intentionally ignored with explanation, or queried)

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request initiates a refactoring of the 'End of Rains' dialog's definition saving mechanism. The goal is to streamline the user interface by consolidating definition saving options into a single, more integrated control. While the structural changes have been implemented, a critical issue causing the application to freeze upon interaction with the new save control has been identified, and the author is requesting collaborative input to debug and resolve it.

Highlights

• UI Refactoring for Definition Saving: The dedicated 'Definitions' checkbox (ucrChkDefinitions) has been removed and its functionality integrated into the ucrSaveObject control, which now includes a checkbox for saving definitions.
• Known Issue: Application Freezing: The author reports that clicking the new 'Store Definitions' checkbox within the ucrSaveObject causes the entire application to freeze, and is seeking assistance to resolve this.
• UI Layout Adjustments: Numerous UI controls across the dialog have had their positioning, margins, and sizes updated, likely to accommodate the control refactoring or for general UI scaling improvements.
• Conditional OK Button State: The 'OK' button is now disabled if the 'Store Definitions' checkbox is checked but the ucrSaveObject control is not in a complete state, preventing incomplete save operations.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• instat/dlgEndOfRainsSeason.Designer.vb
  • Removed the ucrChkDefinitions control declaration and initialization.
  • Adjusted Location, Margin, and Size properties for numerous UI elements including labels, radio buttons, group boxes, input fields, numeric up-down controls, and the main form.
  • Removed ucrChkDefinitions from the Friend WithEvents declarations.
• instat/dlgEndOfRainsSeason.vb
  • Commented out code related to setting text and parameter conditions for the removed ucrChkDefinitions control.
  • Commented out the linking of ucrChkDefinitions to ucrSaveObject.
  • Updated ucrSaveObject to use SetCheckBoxText instead of SetLabelText, indicating its new checkbox functionality.
  • Commented out the SetRCode call for ucrChkDefinitions.
  • Added logic to TestOKEnabled to disable the OK button if ucrSaveObject.ucrChkSave is checked but the ucrSaveObject is incomplete.
  • Modified the condition for adding save definition options to check ucrSaveObject.ucrChkSave.Checked instead of ucrChkDefinitions.Checked.
  • Introduced a new event handler ucrSaveObject_ControlValueChanged to call AddSaveDefinitionOptions.
  • Added ucrSaveObject.ControlContentsChanged to the CoreControls_ControlContentsChanged event handler.
  • Removed the Handles clause from ucrChkDefinitions_ControlValueChanged.

Activity

• The author, Emmanuel-Afrifa, created this pull request to address issue Update the End of Rains/Season Save Control for Definitions #10188.
• The author has identified a critical bug where clicking the new save definitions checkbox freezes the application and is seeking assistance from other team members (@lilyclements, @rdstern, @berylwaswa, @Patowhiz, @N-thony, @Vitalis95).
• The pull request description includes a developer testing checklist, indicating a structured approach to testing, though some items are unchecked due to the identified issue.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to replace a separate checkbox for saving definitions with the integrated checkbox in the ucrSave control. However, as you've noted, this has introduced an application freeze. My review confirms that this is due to an infinite recursion in the ucrSaveObject_ControlValueChanged event handler. I've provided a critical comment with a suggested fix to resolve this issue by using a re-entrancy guard. Additionally, I've included a minor suggestion to remove some now-unused code for better maintainability. The other changes related to replacing the old checkbox control seem correct.

[lilyclements]

@ARMSTRONGOPONDO can you review the VB code here?

[lilyclements]

@Emmanuel-Afrifa Works well! Only thing is that the saved definitions name doesn't update if I change it from the default.

(e.g, check to save the new definition, change the name, and then open Prepare > R Objects > View, and the object has it's default name not the name I set it to. This is the case at least for End Season - I assume for End Rains too)

[ARMSTRONGOPONDO]

@Emmanuel-Afrifa can you please mention the datasets you used to test the changes with

[Emmanuel-Afrifa]

@ARMSTRONGOPONDO I used the guinea_two_stations dataset from the R-Instat climatic datasets
So
Import from Library -> Load from instat collection -> Climatic -> Guinea -> guinea_two_stations

[Emmanuel-Afrifa]

@lilyclements Please, I'll look into that right away. Thank you.

[ARMSTRONGOPONDO]

I have tested the change-save-options-control-end-rains branch @lilyclements and @Emmanuel-Afrifa

The infinite recursion (UI freeze) is resolved. I can now interact with the "Save Object" control without the application crashing. The re-entrancy guard in ucrSaveObject_ControlValueChanged is working correctly.

1. Object Name Not Saving:
   As noted by @lilyclements , the control ignores custom names. If I change the name from the default (e.g., to my_def), the generated R script still uses the default name. It seems AddSaveDefinitionOptions() resets the value on every event trigger.

Please modify AddSaveDefinitionOptions to preserve user input and fix the R script generation logic.

2. Blocking R Errors
   Although the infinite loop is resolved and the script is generated, I experienced the following blocking errors when running the analysis (see attached screenshots):

• Error in data_book$define_as_climatic(...) : unused argument (overwrite = FALSE)
  The overwrite argument appears to be invalid for this function.
• Error in self$apply_instat_calculation(...) : Cannot merge sub calculations as there is no key defined...
  This likely happens because the climatic definition (above) failed, so the keys weren't set on the dataframe.
• Error: attempt to apply non-function
  The function get_end_rains_definition could not be found or applied.

can someone else confirm this maybe am the only one experiencing these errors @berylwaswa

[lilyclements]

@ARMSTRONGOPONDO great. To your R errors can you update the databook and epicsawrap and then try again? I believe the errors are coming from you having a slightly outdated package version.

[Emmanuel-Afrifa]

@lilyclements @ARMSTRONGOPONDO

I've fixed the overwriting of the save name by refactoring the logic that sets the prefix into a separate sub and calling it only in the SetDefaults and when the radio button changes.

Please can you confirm if it works as expected? Thank you.

[ARMSTRONGOPONDO]

@ARMSTRONGOPONDO great. To your R errors can you update the databook and epicsawrap and then try again? I believe the errors are coming from you having a slightly outdated package version.

@Emmanuel-Afrifa
thanks I have updated it and am not exprienocing the same errors @lilyclements

[ARMSTRONGOPONDO]

@Emmanuel-Afrifa I can confirm that store definitions custom name now works okay from my end, I was able to change the name without it changing to the default name

[lilyclements]

@Emmanuel-Afrifa it is working well. It takes the new name now which is a relief, but the name has reverted back to the default name when reopening. As you said, this is an issue elsewhere like on the plot dialogs. Can you put this in an issue?

In the meantime I'll approve and merge. Great work!