Enable overrides in Land node

[barneydobson]

@liuly12 OK I've finished the example for how to do this in the WTW/WWTW node see: #70

We'll use this issue to discuss any additions required for the Land node override behaviour, which will of course be the most complex. Feel free to make a branch and PR when you're ready. Though possibly we will want to have multiple branches for the different surfaces - I'm easy.

[barneydobson]

I guess also NutrientPool is covered here..

[liuly12]

@barneydobson Do you think self.data_input_dict should be revised as a property as well?

[barneydobson]

Good question, I was thinking about that in the context of Catchment. It's quite a specific format to allow someone to overwrite to?

[barneydobson]

In addition, if we do, it should probably run some checking to make sure the appropriate keys are included

[barneydobson]

I'd say don't put code for data_input_dict overrides for Land. It can be tackled in its own issue at a later date if necessary, and centralised for all nodes that read data

[liuly12]

Commits and pull request in a new branch #76

[barneydobson]

Add overrides to residence_times with commit 31808b8

@liuly12 think this should be on its own branch + PR

[liuly12]

Following the discussion in 82c7455#r1509275455. Still not very clear what would be the difference between using property and not using property. It seems to me that most overrides can be achieved without using property.

def apply_overrides(self, overrides = Dict[str, Any]):
        self.surface = overrides.pop("surface", 
                                  self.surface)
        self.area = overrides.pop("area", 
                                  self.area)
        self.depth = overrides.pop("depth", 
                                  self.depth)
        self.capacity = self.area * self.depth

[barneydobson]

So in this example, I agree that surface, area and depth should be set as above, but if capacity is a property:

@property
def capacity(self):
  return self.area * self.depth

Then when you try and set capacity it will throw an error. If we want a user to only ever define area and depth and never capacity - this is a handy way to force that and ensure that the value of capacity is always up to date. We will probably have to add something in __init__ that pings a deprecation message saying that capacity is now unused and that it is derived from depth and area to maintain compatibility with all the various config files we have.

In this sense - the rule is that we reserve @property for when the attribute is directly derived from other attributes of the class. Or (as with say percent_solids in WWTW) when that attribute requires us to recalculate something else (i.e., in this example process_parameters["volume"]["constant"])

[liuly12]

I try to add the code below in the Surface

@property
def capacity(self):
  return self.area * self.depth

and then I get an error when initialising all the superclasses of Surface, using ImperviousSurface as an example:

    surface = ImperviousSurface(
  File "c:\users\leyan\documents\github\wsi\wsimod\nodes\land.py", line 599, in __init__
    super().__init__(depth=pore_depth, **kwargs)
  File "c:\users\leyan\documents\github\wsi\wsimod\nodes\land.py", line 366, in __init__
    super().__init__(capacity=capacity,
  File "c:\users\leyan\documents\github\wsi\wsimod\nodes\nodes.py", line 1136, in __init__
    Tank.__init__(self, **kwargs)
  File "c:\users\leyan\documents\github\wsi\wsimod\nodes\nodes.py", line 770, in __init__
    self.capacity = capacity
AttributeError: can't set attribute 'capacity'

I guess this error basically says that I'm making capacity as a property of Surface so that it cannot be initialised from its subclass Tank based on the current code?

[barneydobson]

OK it is clearly more complicated than expected - in which case just leaving it as is is probably more sensible and using your first solution:
self.capacity = self.area * self.depth

[barneydobson]

fixed by #76