Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/dei 58 new time add as coord #18

Merged
merged 62 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
88aeb32
DEI-49 Add dataset util functions. Add tests for all utils
mKlapwijk Feb 22, 2023
0045148
DEI-49 Add non working rename function to utils
mKlapwijk Feb 22, 2023
7e1275b
DEI-49 Remove print statements
mKlapwijk Feb 22, 2023
f4bdc1c
Added copying of location and mesh attributes
HiddeElzinga Feb 22, 2023
f0053c2
Added attributes to variable assignment
HiddeElzinga Feb 22, 2023
c5f248b
DEI-39 add merge datasets routine to utils
mKlapwijk Feb 24, 2023
ec50362
DEI-49 Add function to merge list of datasets
mKlapwijk Feb 24, 2023
785e09a
Merge branch 'poc/add-location-mesh-attributes' into DEI-49-Select-va…
mKlapwijk Feb 24, 2023
20da2cc
DEI-39 Fix utils tests. Add mapping to modelbuilder. Add intermediate…
mKlapwijk Feb 24, 2023
bff1034
DEI-49 Make list of input and output variable names. Add system names…
mKlapwijk Feb 24, 2023
7f98225
DEI-49 make output_names also a list. Reorder flatten and remove dupl…
mKlapwijk Feb 24, 2023
186f24d
DEI-49. Some cleaning. Comment not working test.
mKlapwijk Feb 24, 2023
3142969
DEI-49 Remove all unneeded variables
mKlapwijk Feb 24, 2023
7d31d5d
DEI-49 Refactor
mKlapwijk Feb 24, 2023
8bd4db0
DEI-49 Remove now obsolete system mapping
mKlapwijk Feb 24, 2023
d924a0a
DEI-49 Do renaming on output dataset
mKlapwijk Feb 24, 2023
0462ffc
DEI-49 Add comment
mKlapwijk Feb 24, 2023
c8a3ba6
DEI-49 Add mapping keys to variables not to be removed
mKlapwijk Feb 24, 2023
14d6c47
Merge branch 'main' into DEI-49-Select-variables-to-be-included-in-ou…
mKlapwijk Feb 24, 2023
b151176
DEI-49 Add yaml files to git ignore
mKlapwijk Feb 27, 2023
65dee6a
Merge branch 'main' into DEI-49-Select-variables-to-be-included-in-ou…
mKlapwijk Feb 27, 2023
2903ad7
Added mapping validation
HiddeElzinga Feb 28, 2023
cc3cb73
Added test for remove_duplicates_from_list
HiddeElzinga Mar 1, 2023
88ea44a
formatting
mKlapwijk Mar 1, 2023
cf9dc9e
Add description of what is returned
mKlapwijk Mar 1, 2023
2da733c
Add mapping to validation test
mKlapwijk Mar 3, 2023
0ed8ffd
Move mapping to proper location in test
mKlapwijk Mar 3, 2023
06b00f2
Extend test for duplicate mapping
mKlapwijk Mar 3, 2023
fd3d516
Add description of test
mKlapwijk Mar 3, 2023
86d10a3
Merge branch 'main' into feature/DEI-49-Select-variables-to-be-includ…
mKlapwijk Mar 3, 2023
73cd39d
Fix test after merge
mKlapwijk Mar 3, 2023
428aa9f
DEI-49 Reorder and extend tests for mapping validation
mKlapwijk Mar 6, 2023
e38f581
check on dependent variabales and ugrid conventions
CindyvdVries Mar 6, 2023
0aeca74
add tests
CindyvdVries Mar 8, 2023
5a283ea
fix flake8
CindyvdVries Mar 8, 2023
b3265fe
Minor changes (typos and a couple of variable type).
Davidrag Mar 8, 2023
817bb11
Corrections after code review
mKlapwijk Mar 9, 2023
b7caadf
fix long_name and standard_name
CindyvdVries Mar 9, 2023
1f418af
Merge branch 'feature/DEI-49-Select-variables-to-be-included-in-outpu…
CindyvdVries Mar 9, 2023
e0a38b1
Added couple missing tests and fixed typos.
Davidrag Mar 10, 2023
ddf3d13
Fixed Pylance smell.
Davidrag Mar 10, 2023
26e6c73
Merge branch 'feature/DEI-49-Select-variables-to-be-included-in-outpu…
CindyvdVries Mar 10, 2023
db78e76
Update test_rule_based_model.py
CindyvdVries Mar 10, 2023
a8abc8d
Small fixes in review
mKlapwijk Mar 10, 2023
12d883e
copy coords from rule
CindyvdVries Mar 10, 2023
ab0b4f5
Update time_aggregation_rule.py
CindyvdVries Mar 10, 2023
3325039
move recursive funciton to dataset_utiils
CindyvdVries Mar 10, 2023
ee30423
fix flake8
CindyvdVries Mar 10, 2023
6184908
Update test_dataset_utils.py
CindyvdVries Mar 10, 2023
72ce734
update documentation
CindyvdVries Mar 10, 2023
173dce2
Merge branch 'main' into feature/DEI-44-ugrid-conventions
CindyvdVries Mar 10, 2023
93eff3f
Merge branch 'main' into feature/DEI-44-ugrid-conventions
CindyvdVries Mar 10, 2023
39a1975
fix merge provlems
CindyvdVries Mar 10, 2023
264ef33
Merge branch 'feature/DEI-44-ugrid-conventions' into feature/DEI-58-n…
CindyvdVries Mar 10, 2023
4687a4c
add test
CindyvdVries Mar 14, 2023
e89513b
fix flake8
CindyvdVries Mar 14, 2023
abd8558
Update time_aggregation_rule.py
CindyvdVries Mar 14, 2023
f564d29
Update rule_based_model.py
CindyvdVries Mar 14, 2023
77c0640
Merge branch 'main' into feature/DEI-58-new-time-add-as-coord
CindyvdVries Mar 14, 2023
11b47fe
fix test based on review
IoannaMi Mar 22, 2023
4b96052
Merge branch 'main' into feature/DEI-58-new-time-add-as-coord
IoannaMi Mar 22, 2023
461ba6c
fix flake8 error
IoannaMi Mar 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions decoimpact/business/entities/rule_based_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ def initialize(self, logger: ILogger) -> None:
self._output_dataset = _du.create_composed_dataset(
self._input_datasets, self._make_output_variables_list(), self._mappings
)

self._rule_processor = RuleProcessor(self._rules, self._output_dataset)
success = self._rule_processor.initialize(logger)

Expand All @@ -109,7 +108,9 @@ def execute(self, logger: ILogger) -> None:
if self._rule_processor is None:
raise RuntimeError("Processor is not set, please initialize model.")

self._rule_processor.process_rules(self._output_dataset, logger)
self._output_dataset = self._rule_processor.process_rules(
self._output_dataset, logger
)

def finalize(self, logger: ILogger) -> None:
"""Finalizes the model"""
Expand Down
13 changes: 12 additions & 1 deletion decoimpact/business/entities/rule_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ def initialize(self, logger: ILogger) -> bool:

return success

def process_rules(self, output_dataset: _xr.Dataset, logger: ILogger) -> None:
def process_rules(
self, output_dataset: _xr.Dataset, logger: ILogger
) -> _xr.Dataset:
"""Processes the rules defined in the initialize method
and adds the results to the provided output_dataset.

Expand All @@ -89,7 +91,16 @@ def process_rules(self, output_dataset: _xr.Dataset, logger: ILogger) -> None:
rule_result.dims,
rule_result.values,
rule_result.attrs,
rule_result.coords,
)
for coord_key in rule_result.coords:
# the coord_key is overwritten in case we don't have the if
# statement below
if coord_key not in output_dataset.coords:
output_dataset = output_dataset.assign_coords(
{coord_key: rule_result[coord_key]}
)
return output_dataset

def _create_rule_sets(
self,
Expand Down
6 changes: 6 additions & 0 deletions decoimpact/business/entities/rules/time_aggregation_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ def execute(self, value_array: _xr.DataArray, logger: ILogger) -> _xr.DataArray:
if value:
result[result_time_dim_name].attrs[key] = value

result = result.assign_coords({
result_time_dim_name: result[result_time_dim_name]
})
result[result_time_dim_name].attrs['long_name'] = result_time_dim_name
result[result_time_dim_name].attrs['standard_name'] = result_time_dim_name

return result

def _perform_operation(self, aggregated_values: DataArrayResample) -> _xr.DataArray:
Expand Down
50 changes: 50 additions & 0 deletions tests/business/entities/test_rule_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
IMultiArrayBasedRule,
)
from decoimpact.business.entities.rules.i_rule import IRule
from decoimpact.business.entities.rules.time_aggregation_rule import TimeAggregationRule
from decoimpact.crosscutting.i_logger import ILogger
from decoimpact.data.api.i_time_aggregation_rule_data import ITimeAggregationRuleData


def _create_test_rules() -> List[IRule]:
Expand Down Expand Up @@ -384,6 +386,54 @@ def test_process_rules_throws_exception_for_unsupported_rule():
assert exception_raised.args[0] == expected_message


def test_process_rules_copies_multi_coords_correctly():
"""Tests if during processing the coords are copied to the output array
and there are no duplicates."""

# Arrange
output_dataset = _xr.Dataset()
output_dataset["test"] = _xr.DataArray([32, 94, 9])

logger = Mock(ILogger)
rule = Mock(IArrayBasedRule)
rule_2 = Mock(IArrayBasedRule)

result_array = _xr.DataArray([27, 45, 93])
result_array = result_array.assign_coords({"test": _xr.DataArray([2, 4, 5])})

result_array_2 = _xr.DataArray([1, 2, 93])
result_array_2 = result_array.assign_coords({"test": _xr.DataArray([2, 4, 5])})

rule.input_variable_names = ["test"]
rule.output_variable_name = "output"
rule.execute.return_value = result_array

rule_2.input_variable_names = ["test"]
rule_2.output_variable_name = "output_2"
rule_2.execute.return_value = result_array_2

processor = RuleProcessor([rule, rule_2], output_dataset)

# Act
assert processor.initialize(logger)
result_dataset = processor.process_rules(output_dataset, logger)

# Assert
assert "test" in result_dataset.coords
# compare coords at the level of variable
result_array_coords = result_array.coords["test"]
result_output_var_coords = result_dataset.output.coords["test"] # output variable
assert (result_output_var_coords == result_array_coords).all()

# compare coords at the level of dataset /
# check if the coordinates are correctly copied to the dataset
result_dataset_coords = result_dataset.coords["test"]
assert (result_output_var_coords == result_dataset_coords).all()

# check if havnig an extra rule with coordinates then they are not copy pasted too
assert len(result_dataset.output.coords) == 1


def test_execute_rule_throws_error_for_unknown_input_variable():
"""Tests that trying to execute a rule with an unknown input variable
throws an error, and the error message."""
Expand Down