Skip to content

Commit

Permalink
Straighten out error handling via a thread-local (but otherwise globa…
Browse files Browse the repository at this point in the history 
…l) context (#1327)

* Introduce an ErrorContext manager.

* Content._carry won't be needing its 3rd argument anymore.

* Replace NestedIndexError with nested_indexerror.

* Remove NestedIndexError entirely. Using ErrorContext now.

* Move error-handling to ak._v2._util.

* Every exception should pass through ak._v2._util.error.

* ak._v2._util.error can now check for contexts.

* Defined error format for operations.

* Now every operation sets the OperationErrorContext.

* Now they do (I missed a big batch last time).

* Avoid reentering the OperationErrorContext.

* Make reducers non-reenterant, too.

* Merged with main.

* chore: add a custom flake8 check

* Fix that one NotImplementedError.

Co-authored-by: Henry Schreiner <[email protected]>
  • Loading branch information
@jpivarski @henryiii
jpivarski and henryiii authored Mar 3, 2022
1 parent 62e15fd commit 8a61c88
Show file tree
Hide file tree
Showing 148 changed files with 3,532 additions and 1,687 deletions.
57 changes: 57 additions & 0 deletions dev/flake8_awkward.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import ast
import sys
from typing import NamedTuple, Iterator


class Flake8ASTErrorInfo(NamedTuple):
line_number: int
offset: int
msg: str
cls: type # unused


class Visitor(ast.NodeVisitor):
msg = "AK101 exception must be wrapped in ak._v2._util.*error"

def __init__(self):
self.errors: list[Flake8ASTErrorInfo] = []

def visit_Raise(self, node):
if isinstance(node.exc, ast.Call):
if isinstance(node.exc.func, ast.Attribute):
if node.exc.func.attr in {"error", "indexerror"}:
return
if node.exc.func.id in {"ImportError"}:
return

self.errors.append(
Flake8ASTErrorInfo(node.lineno, node.col_offset, self.msg, type(self))
)


class AwkwardASTPlugin:
name = "flake8_awkward"
version = "0.0.0"

def __init__(self, tree: ast.AST) -> None:
self._tree = tree

def run(self) -> Iterator[Flake8ASTErrorInfo]:
visitor = Visitor()
visitor.visit(self._tree)
yield from visitor.errors


def main(path):
with open(path) as f:
code = f.read()

node = ast.parse(code)
plugin = AwkwardASTPlugin(node)
for err in plugin.run():
print(f"{path}:{err.line_number}:{err.offset} {err.msg}")


if __name__ == "__main__":
for item in sys.argv[1:]:
main(item)
23 changes: 16 additions & 7 deletions setup.cfg
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,23 @@ numba_extensions =
init = awkward._connect._numba:register

[flake8]
extend-select = C,B,B9,T
extend-select = C,B,B9,T,AK1
extend-ignore = E203,E501,E266
max-complexity = 100
exclude = studies, pybind11, rapidjson, dlpack, docs-*, src/awkward/_typeparser/generated_parser.py, awkward/_typeparser/generated_parser.py
per-file-ignores =
tests/*: T
dev/*: T
setup.py: T
localbuild.py: T
src/awkward/__init__.py: E402, F401, F403
./awkward/__init__.py: E402, F401, F403
tests/*: T, AK1
dev/*: T, AK1
setup.py: T, AK1
localbuild.py: T, AK1
src/awkward/__init__.py: E402, F401, F403, AK1
awkward/__init__.py: E402, F401, F403, AK1
src/awkward/_v2/_connect/numba/*: AK1
src/awkward/[!_]*: AK1
src/awkward/_[!v]*: AK1

[flake8:local-plugins]
extension =
AK1 = flake8_awkward:AwkwardASTPlugin
paths =
./dev/
84 changes: 51 additions & 33 deletions src/awkward/_v2/_broadcasting.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ def checklength(inputs, options):
length = inputs[0].length
for x in inputs[1:]:
if x.length != length:
raise ValueError(
"cannot broadcast {} of length {} with {} of length {}{}".format(
type(inputs[0]).__name__,
length,
type(x).__name__,
x.length,
in_function(options),
raise ak._v2._util.error(
ValueError(
"cannot broadcast {} of length {} with {} of length {}{}".format(
type(inputs[0]).__name__,
length,
type(x).__name__,
x.length,
in_function(options),
)
)
)

Expand Down Expand Up @@ -298,10 +300,14 @@ def continuation():
if length is None:
length = tagslist[-1].shape[0]
elif length != tagslist[-1].shape[0]:
raise ValueError(
"cannot broadcast UnionArray of length {} "
"with UnionArray of length {}{}".format(
length, tagslist[-1].shape[0], in_function(options)
raise ak._v2._util.error(
ValueError(
"cannot broadcast UnionArray of length {} "
"with UnionArray of length {}{}".format(
length,
tagslist[-1].shape[0],
in_function(options),
)
)
)
assert length is not None
Expand Down Expand Up @@ -454,10 +460,12 @@ def continuation():
elif x.size == maxsize:
nextinputs.append(x.content[: x.length * x.size])
else:
raise ValueError(
"cannot broadcast RegularArray of size "
"{} with RegularArray of size {} {}".format(
x.size, maxsize, in_function(options)
raise ak._v2._util.error(
ValueError(
"cannot broadcast RegularArray of size "
"{} with RegularArray of size {} {}".format(
x.size, maxsize, in_function(options)
)
)
)
else:
Expand Down Expand Up @@ -573,9 +581,11 @@ def continuation():
for x in outcontent
)
else:
raise AssertionError(
"unexpected offsets, starts: {}, {}".format(
type(offsets), type(starts)
raise ak._v2._util.error(
AssertionError(
"unexpected offsets, starts: {}, {}".format(
type(offsets), type(starts)
)
)
)

Expand Down Expand Up @@ -641,29 +651,35 @@ def continuation():
# Any RecordArrays?
elif any(isinstance(x, RecordArray) for x in inputs):
if not options["allow_records"]:
raise ValueError(f"cannot broadcast records{in_function(options)}")
raise ak._v2._util.error(
ValueError(f"cannot broadcast records{in_function(options)}")
)

fields, length, istuple = None, None, True
for x in inputs:
if isinstance(x, RecordArray):
if fields is None:
fields = x.fields
elif set(fields) != set(x.fields):
raise ValueError(
"cannot broadcast records because fields don't "
"match{}:\n {}\n {}".format(
in_function(options),
", ".join(sorted(fields)),
", ".join(sorted(x.fields)),
raise ak._v2._util.error(
ValueError(
"cannot broadcast records because fields don't "
"match{}:\n {}\n {}".format(
in_function(options),
", ".join(sorted(fields)),
", ".join(sorted(x.fields)),
)
)
)
if length is None:
length = x.length
elif length != x.length:
raise ValueError(
"cannot broadcast RecordArray of length {} "
"with RecordArray of length {}{}".format(
length, x.length, in_function(options)
raise ak._v2._util.error(
ValueError(
"cannot broadcast RecordArray of length {} "
"with RecordArray of length {}{}".format(
length, x.length, in_function(options)
)
)
)
if not x.is_tuple:
Expand Down Expand Up @@ -695,9 +711,11 @@ def continuation():
)

else:
raise ValueError(
"cannot broadcast: {}{}".format(
", ".join(repr(type(x)) for x in inputs), in_function(options)
raise ak._v2._util.error(
ValueError(
"cannot broadcast: {}{}".format(
", ".join(repr(type(x)) for x in inputs), in_function(options)
)
)
)

Expand All @@ -717,7 +735,7 @@ def continuation():
elif result is None:
return continuation()
else:
raise AssertionError(result)
raise ak._v2._util.error(AssertionError(result))


def broadcast_and_apply(
Expand Down
12 changes: 6 additions & 6 deletions src/awkward/_v2/_connect/cling.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def togenerator(form):
return UnionArrayGenerator.from_form(form)

else:
raise AssertionError(f"unrecognized Form: {type(form)}")
raise ak._v2._util.error(AssertionError(f"unrecognized Form: {type(form)}"))


class Generator:
Expand All @@ -127,7 +127,7 @@ def form_from_identifier(cls, form):
if not form.has_identifier:
return None
else:
raise NotImplementedError("TODO: identifiers in C++")
raise ak._v2._util.error(NotImplementedError("TODO: identifiers in C++"))

def class_type_suffix(self, key):
return (
Expand Down Expand Up @@ -393,7 +393,7 @@ def __init__(self, index_type, content, identifier, parameters):
elif index_type == "i64":
self.index_type = "int64_t"
else:
raise AssertionError(index_type)
raise ak._v2._util.error(AssertionError(index_type))
self.content = content
self.identifier = identifier
self.parameters = parameters
Expand Down Expand Up @@ -529,7 +529,7 @@ def __init__(self, index_type, content, identifier, parameters):
elif index_type == "i64":
self.index_type = "int64_t"
else:
raise AssertionError(index_type)
raise ak._v2._util.error(AssertionError(index_type))
self.content = content
self.identifier = identifier
self.parameters = parameters
Expand Down Expand Up @@ -604,7 +604,7 @@ def __init__(self, index_type, content, identifier, parameters):
elif index_type == "i64":
self.index_type = "int64_t"
else:
raise AssertionError(index_type)
raise ak._v2._util.error(AssertionError(index_type))
self.content = content
self.identifier = identifier
self.parameters = parameters
Expand Down Expand Up @@ -1061,7 +1061,7 @@ def __init__(self, index_type, contents, identifier, parameters):
elif index_type == "i64":
self.index_type = "int64_t"
else:
raise AssertionError(index_type)
raise ak._v2._util.error(AssertionError(index_type))
self.contents = tuple(contents)
self.identifier = identifier
self.parameters = parameters
Expand Down
22 changes: 12 additions & 10 deletions src/awkward/_v2/_connect/numpy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,11 @@ def action(inputs, **ignore):
error_message.append(type(x).__name__)
else:
error_message.append(type(x).__name__)
raise TypeError(
"no {}.{} overloads for custom types: {}".format(
type(ufunc).__module__, ufunc.__name__, ", ".join(error_message)
raise ak._v2._util.error(
TypeError(
"no {}.{} overloads for custom types: {}".format(
type(ufunc).__module__, ufunc.__name__, ", ".join(error_message)
)
)
)

Expand Down Expand Up @@ -286,9 +288,9 @@ def unary_action(layout, **ignore):
# if first == -1:
# first = len(Ai)
# elif first != len(Ai):
# raise ValueError(
# raise ak._v2._util.error(ValueError(
# "one of the left matrices in np.matmul is not rectangular"
# )
# ))
# if first == -1:
# first = 0
# rowsA = len(A)
Expand All @@ -299,19 +301,19 @@ def unary_action(layout, **ignore):
# if first == -1:
# first = len(Bi)
# elif first != len(Bi):
# raise ValueError(
# raise ak._v2._util.error(ValueError(
# "one of the right matrices in np.matmul is not rectangular"
# )
# ))
# if first == -1:
# first = 0
# rowsB = len(B)
# colsB = first

# if colsA != rowsB:
# raise ValueError(
# raise ak._v2._util.error(ValueError(
# u"one of the pairs of matrices in np.matmul do not match shape: "
# u"(n \u00d7 k) @ (k \u00d7 m)"
# )
# ))

# total_outer += 1
# total_inner += rowsA
Expand Down Expand Up @@ -355,7 +357,7 @@ def unary_action(layout, **ignore):


def action_for_matmul(inputs):
raise NotImplementedError
raise ak._v2._util.error(NotImplementedError)


# def action_for_matmul(inputs):
Expand Down
Loading

0 comments on commit 8a61c88

Please sign in to comment.