PI-3677: Remove regex from standard names check #3679
Conversation
lib/iris/tests/unit/cube_coord_common/test_get_valid_standard_name.py
Outdated
Show resolved
Hide resolved
|
I updated master, but final changes must be propagated to 2.4.x to fix ESMValGroup/ESMValTool#1561 |
lib/iris/_cube_coord_common.py
Outdated
| else: | ||
| name_is_valid = False |
There was a problem hiding this comment.
@jvegasbsc There is no need for the else as name_is_valid is not used outside the scope of the if statement
| @@ -19,10 +19,26 @@ class Test(tests.IrisTest): | |||
| def setUp(self): | |||
| self.emsg = "'{}' is not a valid standard_name" | |||
|
|
|||
| def test_none_is_valid(self): | |||
There was a problem hiding this comment.
@jvegasbsc I'm going to be a tad fussy here, sorry... None is valid in the sense that no exception is raised by get_valid_standard_name, but obviously it's not an actual valid standard name.
Could we change the intent of this test name to be something like test_pass_thru_none instead?
| name = None | ||
| self.assertEqual(get_valid_standard_name(name), name) | ||
|
|
||
| def test_empty_is_not_valid(self): |
There was a problem hiding this comment.
@jvegasbsc Same here, change test name to be test_pass_thru_empty...
| name = '' | ||
| self.assertEqual(get_valid_standard_name(name), name) | ||
|
|
||
| def test_only_whitespace_is_not_valid(self): |
There was a problem hiding this comment.
@jvegasbsc Same here, change test name to be test_pass_thru_whitespace...
| def test_valid_standard_name(self): | ||
| name = "air_temperature" | ||
| self.assertEqual(get_valid_standard_name(name), name) | ||
|
|
||
| def test_standard_name_alias(self): | ||
| name = "atmosphere_optical_thickness_due_to_pm1_ambient_aerosol" | ||
| self.assertEqual(get_valid_standard_name(name), name) |
There was a problem hiding this comment.
@jvegasbsc Nice, covers your test case, thanks! 👍
There was a problem hiding this comment.
@jvegasbsc Awesome PR, thanks @jvegasbsc.
I've suggested some minor tweaks, so if you could service them then we'll get this merged asap.
Also, you may need to rebase the PR to pick up some other changes to fix some tests that are failing in travis-ci.
Thanks 👍
|
@jvegasbsc almost there... you just have a couple of black, version 19.10b0
would reformat /home/travis/build/SciTools/iris/lib/iris/_cube_coord_common.py
would reformat /home/travis/build/SciTools/iris/lib/iris/tests/unit/cube_coord_common/test_get_valid_standard_name.py
Oh no! 💥 💔 💥We've now adopted the black coding standard for Alternatively, you can could also install |
|
Done! |
|
@jvegasbsc Awesome, thanks for your patience and fixing this issue, much appreciated 👍 |
* Remove regex from standard names check * remove duplicated test * Added test cases for none, empty and only spaces * Remove duplicated test * Address comments * Blackify code
The regex in
get_valid_standard_namewas an overkill as the function was already checking the values against a list that contained all the possible values.This pull request replaces it with a simple split and fixes the bug that was making iris not to accept
atmosphere_optical_thickness_due_to_pm1_ambient_aerosolas a valid standard_nameFixes #3677