Clearer error message when using concatenate_cube on multi-variable cubes list

[tinyendian]

🚀 Pull Request

Description

This pull request addresses issue #4342: cube method concatenate_cube() can produce confusing error messages, reporting that var1 != var1 when a user (erroneously) attempts to concatenate a multi-variable cube list consisting of, e.g., multiple time steps of var1 and var2 with cubes = [var1, var1, var2, ...].

The reason for this behaviour is that the error message in the method quotes the first two elements of list names, which can be identical in the above scenario. This pull request replaces names with list unique_names when generating the error message - the latter list is used as a criterion for concatenate_cube() to reject concatenation, and it is guaranteed to produce non-identical variable names. This should give the user better feedback on why concatenation failed.

It should not be necessary to handle the cases of length-zero or length-1 list, since len(unique_names) should always be >=2, given that the method will fail earlier on an empty cube list, and it will accept length 1 for concatenation.

---

Consult Iris pull request check list

[tinyendian]

Note that I just signed the SciTools CLA.

[rcomer]

Hi @tinyendian, thanks for your contribution!

We have some unit tests that check concatenate throws the right error in various circumstances. Would you be happy to add a test to cover the case you've highlighted? [Edit: see next comment about where such a test should go].

Also, so that we can advertise your contribution in the next release, could you add an entry under "Bugs Fixed" in the whatsnew/latest.rst file? There is guidance about writing a "whatsnew" entry here.

Do let us know if any of this isn't clear, or if you need help with anything.

[rcomer]

Sorry, I just realised I pointed to the wrong set of tests in my last comment 🤦‍♀️. The "names differ" error is raised within CubeList.concatenate_cube() itself, so a test to cover this case would need to be added to this class.

[tinyendian]

Hi @rcomer, I added a unit test that replicates the multi-variable scenario mentioned in the issue and checks the error message, as well as a "whatsnew" entry in the list of bug fixes. I hope that both match the required styles.

The doctests and link check are failing with error message "Unknown target name: "@tinyendian", does my GitHub name need to be added somewhere?

[trexfeathers]

"Unknown target name: "@tinyendian", does my GitHub name need to be added somewhere?

Correct!

iris/docs/src/whatsnew/latest.rst

Lines 145 to 149 in a07e20d

	.. comment
	Whatsnew author names (@github name) in alphabetical order. Note that,
	core dev names are automatically included by the common_links.inc:

See here for a populated example from v3.1:

iris/docs/src/whatsnew/3.1.rst

Lines 300 to 305 in b15386e

	.. comment
	Whatsnew author names (@github name) in alphabetical order. Note that,
	core dev names are automatically included by the common_links.inc:
	.. _@akuhnregnier: https://github.com/akuhnregnier
	.. _@Badboy-16: https://github.com/Badboy-16

[rcomer]

Thanks @tinyendian, the new test looks perfect to me. It fails without the fix and passes with it. Just need to add the GitHub link for your handle, as @trexfeathers shows above, and then I think this will be ready to merge.

[tinyendian]

Thanks, @rcomer, working on it...

[rcomer]

Awesome - thanks @tinyendian!

[tinyendian]

Thanks for your help getting this over the line, @rcomer and @trexfeathers!