Fix error message in variable definition check

[enekomartinmartinez]

Description

This PR should correct the bug of not correctly raising the error message in the variable check of the recipe.

Closes #2312

Link to documentation:

---

Before you get started

• ☝ Create an issue to discuss what you are going to do

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

• 🧪 The new functionality is relevant and scientifically sound
• 🛠 This pull request has a descriptive title and labels
• 🛠 Code is written according to the code quality guidelines
• 🛠 Unit tests have been added
• 🛠 Changes are backward compatible
• 🛠 The list of authors is up to date
• 🛠 All checks below this pull request were successful

---

To help with the number pull requests:

• 🙏 We kindly ask you to review two other open pull requests in this repository

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c8635a8) 93.74% compared to head (27a9d03) 93.78%.
Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2313      +/-   ##
==========================================
+ Coverage   93.74%   93.78%   +0.03%     
==========================================
  Files         238      238              
  Lines       13153    13153              
==========================================
+ Hits        12330    12335       +5     
+ Misses        823      818       -5     

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

[enekomartinmartinez]

I just updated the commit so as not to have the same error with the variable_group. It would be nice to include unity tests for these error messages.

[bouweandela]

Thanks for taking the time to contribute! Looks good, just a minor suggestion to make the error message even more readable.

[bouweandela]

If you'd like to add a unit tests for this, see here for an example of how to write such a test:

ESMValCore/tests/integration/recipe/test_check.py

Line 337 in b0d30f6

def test_invalid_multi_model_ignore_scalar_coords():

[enekomartinmartinez]

Thanks for taking the time to contribute! Looks good, just a minor suggestion to make the error message even more readable.

Thanks for the suggestions, I will also update the error message in similar functions to have something more "uniform".

[bouweandela]

Would you still like to add a test, or shall we merge this as-is?

[enekomartinmartinez]

Hi @bouweandela I have tried to uniformize the RecipeErrors and tests them for:

• diagnostics
• duplicate_datasets
• variable

Note that there are two errors (currently added tests with xfail) that are not raised:

ESMValCore/esmvalcore/_recipe/check.py

Lines 71 to 73 in 6f30fa6

	if 'scripts' not in diagnostic:
	raise RecipeError(
	"Missing scripts section in diagnostic {}".format(name))

ESMValCore/esmvalcore/_recipe/check.py

Lines 84 to 87 in 6f30fa6

	if not script.get('script'):
	raise RecipeError(
	"No script defined for script {} in diagnostic {}".format(
	script_name, name))

This is because the error was raised before by the yamale.validate check.

Therefore, for me, it would make sense to do any of these:

1. Remove the RecipeErrors that are not raised and convert the test to a yamale.validate errors
   or
2. Soften the condition for the yamale.validate, setting required=False for those entries in the recipe_schema.yml and let the RecipeError raise later (which I think is more user friendly).

Let me know which one you prefer

[bouweandela]

I agree that option 2 is more user-friendly. Especially less technical users can get confused by the cryptic error messages coming from Yamale (see e.g. #1080 and #681)

[enekomartinmartinez]

@bouweandela, this is already okay and ready to merge by my side. Please, let me know if you find anything else missing!

[bouweandela]

Would you like to add yourself to the list of authors?

[enekomartinmartinez]

Would you like to add yourself to the list of authors?

Done, thank you!

[valeriupredoi]

excellent work, gents 🍻