Closed spencerahill closed 5 years ago
Hmmm not sure what's up with the stickler comments...all tests are passing on my machine.
Also forgot to mention: the changes to Region aren't strictly needed for this PR, so I shouldn't have put them here. But now that they are here: the motivation was that previously Region.mask_var
only masked based on the lat/lon bounds, not the land mask; the land mask logic was handled in ts
. So I moved that from ts
into mask_var
, with the option to ignore it (which is physically necessary when masking the surface area).
Thanks @spencerahill! I'll look have a look over this over the next few days.
Thanks @spencerkclark. Glad it's on the right track. Will have a bit of time next week to return to this, but frankly might be after that when I can get it over the finish line.
Closes #208
This is an initial stab at the implementation of the custom time reduction methods.
New since the most recent discussion in #208: I implemented a generic Reduction class and two subclasses,
GriddedReduction
andRegionReduction
, in order to handle the fact the regional averaging of time-dependent vertical coordinates.An outstanding questions: how to handle identifying time defined reductions. For the built-in reductions, we can just specify them manually as we have been. But this won't work for custom reductions. And the way we're doing it now based on the string labels seems hackish to me.
Beyond that, I would appreciate an initial review @spencerkclark when you get a chance...does this seem like it's heading in the right direction? I'm 100% receptive to changing any of it. No rush...I'm basically out of spare cycles for the rest of this week.
Also still needed: