Skip to content
This repository was archived by the owner on Oct 24, 2024. It is now read-only.

Add path to error message in map_over_subtree #264

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3ee67fe
test
TomNicholas Oct 23, 2023
f367f0f
implementation
TomNicholas Oct 23, 2023
688dc4a
formatting
TomNicholas Oct 23, 2023
333a4e9
add version check, if not using 3.11 then you just won't get the extr…
TomNicholas Oct 24, 2023
b603cd6
whatsnew
TomNicholas Oct 24, 2023
cce673d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 24, 2023
bfb4fbd
Merge branch 'main' into error_context_map_over_subtree
TomNicholas Oct 24, 2023
1adc58a
Merge branch 'main' into error_context_map_over_subtree
TomNicholas Oct 24, 2023
5a1b8b1
use better helper function
TomNicholas Oct 24, 2023
f41cd95
xfail test, because this does actually work...
TomNicholas Oct 24, 2023
b449c9c
Merge branch 'error_context_map_over_subtree' of https://github.com/T…
TomNicholas Oct 24, 2023
6c72798
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 24, 2023
fc59b0e
clean up after bad merge
TomNicholas Oct 24, 2023
eb4a38e
Merge branch 'error_context_map_over_subtree' of https://github.com/T…
TomNicholas Oct 24, 2023
479c74a
Merge branch 'main' into error_context_map_over_subtree
TomNicholas Oct 24, 2023
33efed1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 24, 2023
a50b7f6
re-enable feature after stupid merge
TomNicholas Oct 24, 2023
5da9479
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 24, 2023
7e8aea1
formatting
TomNicholas Oct 24, 2023
2a9b510
Merge branch 'error_context_map_over_subtree' of https://github.com/T…
TomNicholas Oct 24, 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
24 changes: 23 additions & 1 deletion datatree/mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,15 @@ def _map_over_subtree(*args, **kwargs) -> DataTree | Tuple[DataTree, ...]:
],
)
)
func_with_error_context = _handle_errors_with_path_context(
node_of_first_tree.path
)(func)

# Now we can call func on the data in this particular set of corresponding nodes
results = (
func(*node_args_as_datasets, **node_kwargs_as_datasets)
func_with_error_context(
*node_args_as_datasets, **node_kwargs_as_datasets
)
if not node_of_first_tree.is_empty
else None
)
Expand Down Expand Up @@ -251,6 +256,23 @@ def _map_over_subtree(*args, **kwargs) -> DataTree | Tuple[DataTree, ...]:
return _map_over_subtree


def _handle_errors_with_path_context(path):
"""Wraps given function so that if it fails it also raises path to node on which it failed."""

def decorator(func):
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except Exception as e:
# Add the context information to the error message
e.add_note(f"Raised whilst mapping function over node with path {path}")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding notes to exceptions is a new feature in python 3.11.

Perhaps we should wait before merging this? If I understand xarray's deprecation policy correctly then 3.11 will only be the minimum supported version of python 24 months after 3.11 is released, which is 12 months from now...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use a helper for this:

def add_note(err: BaseException, msg: str) -> None:
    # TODO: remove once python 3.10 can be dropped
    if sys.version_info < (3, 11):
        err.__notes__ = getattr(err, "__notes__", []) + [msg]
    else:
        err.add_note(msg)

raise

return wrapper

return decorator


def _check_single_set_return_values(path_to_node, obj):
"""Check types returned from single evaluation of func, and return number of return values received from func."""
if isinstance(obj, (Dataset, DataArray)):
Expand Down
14 changes: 14 additions & 0 deletions datatree/tests/test_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,20 @@ def times_ten(ds):
result_tree = times_ten(subtree)
assert_equal(result_tree, expected, from_root=False)

def test_error_contains_path_of_offending_node(self, create_test_datatree):
dt = create_test_datatree()
dt["set1"]["bad_var"] = 0
print(dt)

def fail_on_specific_node(ds):
if "bad_var" in ds:
raise ValueError("Failed because 'bar_var' present in dataset")

with pytest.raises(
ValueError, match="Raised whilst mapping function over node /set1"
):
dt.map_over_subtree(fail_on_specific_node)


class TestMutableOperations:
def test_construct_using_type(self):
Expand Down