Minimise duplication of the CDDS default request items

[Zubair Maalick (zmaalick)]

Closes #209 .

PR creation checklist for the developer

• Has <issue_number> above ☝️ been replaced with the issue number?
• Has main been selected as the base branch?
• Does the feature branch name follow the format <issue_number>_<short_description_of_feature>?
• Does the text of the PR title exactly match with the text (not including the issue number) of the issue title?
• Have appropriate reviewers been added to the PR (once it is ready for review)?
• Has the PR been assigned to the developer(s)?
• Have the same labels as on the issue (except for the good first issue label) been added to the PR?
• Has the Climate Model Evaluation Workflow (CMEW) project been added to the PR?
• Has the appropriate milestone been added to the PR?

Definition of Done for the developer

• Does the change in this PR address the above issue / have all acceptance criteria been met?
• Does the change in this PR follow the requirements in the wiki: Developer Guide (including copyrights)?
• Have new tests related to the change been added?
• Do all the GitHub workflow checks pass?
• Do all the tests run locally and pass? (Note: the tests are not run by the GitHub workflow, see wiki: Run the tests locally)
• Has the API documentation (e.g. docstrings in Python modules) related to the change been updated appropriately?
• Has the user documentation (i.e. everything in the doc directory) related to the change been updated appropriately, including the Quick Start section?
• Do the HTML pages render correctly? (See wiki: Build the documentation locally)

PR creation checklist for the reviewer

• Has <issue_number> above ☝️ been replaced with the issue number?
• Has main been selected as the base branch?
• Does the feature branch name follow the format <issue_number>_<short_description_of_feature>?
• Does the text of the PR title exactly match with the text (not including the issue number) of the issue title?
• Have appropriate reviewers been added to the PR (once it is ready for review)?
• Has the PR been assigned to the developer(s)?
• Have the same labels as on the issue (except for the good first issue label) been added to the PR?
• Has the Climate Model Evaluation Workflow (CMEW) project been added to the PR?
• Has the appropriate milestone been added to the PR?

Definition of Done for the reviewer

• Does the change in this PR address the above issue / have all acceptance criteria been met?
• Does the change in this PR follow the requirements in the wiki: Developer Guide (including copyrights)?
• Have new tests related to the change been added?
• Do all the GitHub workflow checks pass?
• Do all the tests run locally and pass? (Note: the tests are not run by the GitHub workflow, see wiki: Run the tests locally)
• Has the API documentation (e.g. docstrings in Python modules) related to the change been updated appropriately?
• Has the user documentation (i.e. everything in the doc directory) related to the change been updated appropriately, including the Quick Start section?
• Do the HTML pages render correctly? (See wiki: Build the documentation locally)

[Naomi Parsons (NParsonsMO)]

The unit test for configure_standardise/bin/test_create_request_file.py::test_create_request fails.

I also think that the defaults file is not supposed to contain user-configurable variables.

[Naomi Parsons (NParsonsMO)]

There are merge conflicts that need to be sorted,

and I assume that this needs to be pre-review (although I'm not sure it's come up yet)?

I've left two comments open to check those lines on merging with main.

All unit tests now passing in all methods.

One copyright change requested.

[Naomi Parsons (NParsonsMO)]

One unit test is failing.

Two prior review comments have not been responded to.

[Zubair Maalick (zmaalick)]

One unit test is failing.

Two prior review comments have not been responded to.

Fix the unittest: ca8d9a0

[Naomi Parsons (NParsonsMO)]

Thanks Zubair Maalick (@zmaalick)!

[Alistair Sellar (alistairsellar)]

Thanks Zubair Maalick (@zmaalick)!

[Emma Hogan (ehogan)]

Thanks Zubair Maalick (@zmaalick) 🥳

[Naomi Parsons (NParsonsMO)]

Sorry, I did not mean to close this, I dropped my mouse!

[Naomi Parsons (NParsonsMO)]

I really like the style of the function CMEW.app.configure_standardise.bin.create_request_file.create_request now, it's very readable.

I like it much more than all the nested information in CMEW/app/configure_recipe/bin/configure_recipe.py, but I know we did more away from using a reference file here... Is that a bit inconsistent?

I will approve if the workflow runs successfully but it seems to be stuck on the MASS step at the minute.

[Naomi Parsons (NParsonsMO)]

I will approve if the workflow runs successfully but it seems to be stuck on the MASS step at the minute.

It has now passed fine and my last approval is still showing.

[Emma Hogan (ehogan)]

Thanks Alistair Sellar (@alistairsellar)! 🥳

I agree with all Naomi Parsons (@NParsonsMO)'s comments on the request_defaults.cfg file; this file, as described in the issue, is for default values only, not those configurable by the user (hence the name). Would it be possible for the request_defaults.cfg file to contain only the default values, please?

[Emma Hogan (ehogan)]

There's a tiny bit of consolidation that can be done here to minimise duplication; set:

model_id = "HadGEM2-GC31-HH"

then use model_id here and for model_id below (when creating the expected dictionary) 👍

[Alistair Sellar (alistairsellar)]

I'm not sure I understand this one. There is (before and after this PR) a duplicate specification of most of the items, e.g. INSTITUTION_ID, ROOT_PROC_DIR. Is MODEL_ID used differently in a way that I haven't spotted?

[Alistair Sellar (alistairsellar)]

Thanks for the explanation (offline). Python vars now added to remove duplicate values: 4435d33

[Alistair Sellar (alistairsellar)]

Thanks for the review Emma Hogan (@ehogan)

Would it be possible for the request_defaults.cfg file to contain only the default values, please?

Yes, that would be cleaner. Implemented in b6ff2a4.

Ready for your re-review.