Overrides bug #140
Overrides bug #140
Conversation
| @@ -440,7 +440,7 @@ def test_data_overrides(self): | |||
| node.get_data_input("temperature"), | |||
| ) | |||
| # test runtime error | |||
| self.assertRaises(RuntimeError, lambda: node.apply_overrides({})) | |||
| self.assertRaises(RuntimeError, lambda: node.apply_overrides({"filename": 123})) | |||
There was a problem hiding this comment.
@liuly12 I have changed this because I'm not sure why the previous line should have triggered an error (surely empty overrides are fine?)
There was a problem hiding this comment.
In theory, empty overrides should be fine. what error it reports?
There was a problem hiding this comment.
Exactly - before my change in the behaviour of Node.apply_overrides it would throw this error. Now it only throws the error when a non-sensible filename is passed
|
|
||
| for surface, override in overrides.get("surfaces", {}).items(): | ||
| for surface, override in overrides.pop("surfaces", {}).items(): |
There was a problem hiding this comment.
This is the fix for the warning bug
| @@ -90,7 +90,7 @@ def apply_overrides(self, overrides: Dict[str, Any] = {}) -> None: | |||
| # overrides data_input_dict | |||
| from wsimod.orchestration.model import read_csv | |||
|
|
|||
| content = overrides.pop("data_input_dict", self.data_input_dict) | |||
| content = overrides.pop("filename", None) | |||
There was a problem hiding this comment.
This is the fix for the data_input_dict bug
There was a problem hiding this comment.
does it only happen for Land or actually it happens in any node that requires a filename?
There was a problem hiding this comment.
I guess any node with a data_input_dict, but since the filename override will get passed up to the Node.apply_overrides via super - it only need to be corrected here
| @@ -1431,6 +1431,14 @@ def test_apply_surface_overrides(tmp_path): | |||
| import yaml | |||
| from wsimod.orchestration.model import Model | |||
There was a problem hiding this comment.
Updates in this file triggered the failing test
Bug found with data input overrides.
Two problems:
filenameis the key used to specify input data atload, notdata_input_dict. This created an error if applying overrides to a node withfilenamein the original config (as would be the case with a typicalLandnode).Surface.apply_overrideswas being called after theLand.super().apply_overridescall - thus the{"surface":overrides}were present in thesupercall and theNo override behaviour defined for...warning was being printed.These have been resolved by:
data_input_dictin override keys tofilenameSurface.apply_overridesbefore thesupercall, and calling it withpopso that it wasn't present in theoverridespassed tosuper.