Skip to content

Commit d659ecc

Browse files
timofey-stepanovcopybara-github
timofey-stepanov
authored and
copybara-github
committed
Remove AsKodaError and OperatorEvalError calls from arolla_bridge.
I also remove a few unnecessary clarifications, same as I did for `SimplePointwiseEval` before. PiperOrigin-RevId: 721412060 Change-Id: Icf813a59b05c163c0671ee7409e2a9d8444d312b
1 parent a6504b7 commit d659ecc

File tree

4 files changed

+19
-75
lines changed

4 files changed

+19
-75
lines changed

‎koladata/operators/arolla_bridge.cc‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
#include "koladata/internal/data_item.h"
4242
#include "koladata/internal/dtype.h"
4343
#include "koladata/internal/error_utils.h"
44-
#include "koladata/internal/op_utils/error.h"
4544
#include "koladata/internal/schema_utils.h"
4645
#include "koladata/schema_utils.h"
4746
#include "koladata/shape_utils.h"
@@ -196,15 +195,14 @@ absl::StatusOr<DataSlice> SimpleAggEval(
196195
DCHECK_LE(edge_arg_index, inputs.size());
197196
ASSIGN_OR_RETURN(
198197
auto primary_operand_schema_info,
199-
GetPrimaryOperandSchemaInfo(inputs, primary_operand_indices),
200-
internal::OperatorEvalError(std::move(_), op_name, "invalid inputs"));
198+
GetPrimaryOperandSchemaInfo(inputs, primary_operand_indices));
201199
ASSIGN_OR_RETURN(
202200
(auto [aligned_ds, aligned_shape]),
203201
shape::AlignNonScalars(std::move(inputs)),
204-
internal::OperatorEvalError(std::move(_), op_name,
205-
"cannot align all inputs to a common shape"));
202+
internal::KodaErrorFromCause("cannot align all inputs to a common shape",
203+
std::move(_)));
206204
if (aligned_shape.rank() == 0) {
207-
return internal::OperatorEvalError(op_name, "expected rank(x) > 0");
205+
return absl::InvalidArgumentError("expected rank(x) > 0");
208206
}
209207
auto result_shape = is_agg_into
210208
? aligned_shape.RemoveDims(aligned_shape.rank() - 1)
@@ -237,12 +235,7 @@ absl::StatusOr<DataSlice> SimpleAggEval(
237235
}
238236
auto edge_tv = arolla::TypedValue::FromValue(aligned_shape.edges().back());
239237
typed_refs[edge_arg_index] = edge_tv.AsRef();
240-
ASSIGN_OR_RETURN(
241-
auto result, EvalExpr(op_name, typed_refs),
242-
internal::OperatorEvalError(
243-
std::move(_), op_name,
244-
"successfully converted input DataSlice(s) to DenseArray(s) but "
245-
"failed to evaluate the Arolla operator"));
238+
ASSIGN_OR_RETURN(auto result, EvalExpr(op_name, typed_refs));
246239
if (!output_schema.has_value()) {
247240
// Get the common schema from the primary inputs and output.
248241
ASSIGN_OR_RETURN(auto result_schema, GetResultSchema(result));
@@ -305,9 +298,9 @@ absl::StatusOr<DataSlice> SimplePointwiseEval(
305298
internal::DataItem output_schema,
306299
const std::optional<absl::Span<const int>>& primary_operand_indices) {
307300
DCHECK_GE(inputs.size(), 1);
308-
ASSIGN_OR_RETURN(auto primary_operand_schema_info,
309-
GetPrimaryOperandSchemaInfo(inputs, primary_operand_indices),
310-
internal::AsKodaError(std::move(_)));
301+
ASSIGN_OR_RETURN(
302+
auto primary_operand_schema_info,
303+
GetPrimaryOperandSchemaInfo(inputs, primary_operand_indices));
311304
// All primary inputs are empty-and-unknown. We then skip evaluation and just
312305
// broadcast the first input to the common shape and common schema. It is the
313306
// caller's responsibility to make sure that the non-primary inputs have
@@ -344,8 +337,7 @@ absl::StatusOr<DataSlice> SimplePointwiseEval(
344337
primary_operand_schema_info.first_primitive_schema));
345338
typed_refs.push_back(std::move(ref));
346339
}
347-
ASSIGN_OR_RETURN(auto result, EvalExpr(op_name, typed_refs),
348-
internal::AsKodaError(std::move(_)));
340+
ASSIGN_OR_RETURN(auto result, EvalExpr(op_name, typed_refs));
349341
if (!output_schema.has_value()) {
350342
// Get the common schema from the primary inputs and output.
351343
ASSIGN_OR_RETURN(auto result_schema, GetResultSchema(result));

‎koladata/operators/arolla_bridge_test.cc‎

Lines changed: 6 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,6 @@ TEST(ArollaEval, SimplePointwiseEval) {
182182
status,
183183
StatusIs(absl::StatusCode::kInvalidArgument,
184184
HasSubstr("DataSlice with Struct schema is not supported")));
185-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
186-
EXPECT_TRUE(payload.has_value());
187-
EXPECT_THAT(payload->error_message(),
188-
HasSubstr("DataSlice with Struct schema is not supported"));
189185
}
190186
{
191187
// invalid input: mixed types.
@@ -198,10 +194,6 @@ TEST(ArollaEval, SimplePointwiseEval) {
198194
status,
199195
StatusIs(absl::StatusCode::kInvalidArgument,
200196
HasSubstr("DataSlice with mixed types is not supported")));
201-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
202-
EXPECT_TRUE(payload.has_value());
203-
EXPECT_THAT(payload->error_message(),
204-
HasSubstr("DataSlice with mixed types is not supported"));
205197
}
206198
{
207199
// Invalid input: object.
@@ -212,10 +204,6 @@ TEST(ArollaEval, SimplePointwiseEval) {
212204
EXPECT_THAT(status,
213205
StatusIs(absl::StatusCode::kInvalidArgument,
214206
HasSubstr("DataSlice has no primitive schema")));
215-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
216-
EXPECT_TRUE(payload.has_value());
217-
EXPECT_THAT(payload->error_message(),
218-
HasSubstr("DataSlice has no primitive schema"));
219207
}
220208
{
221209
// Incompatible shapes.
@@ -227,7 +215,7 @@ TEST(ArollaEval, SimplePointwiseEval) {
227215
HasSubstr("shapes are not compatible: JaggedShape(1) "
228216
"vs JaggedShape(3)")));
229217
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
230-
EXPECT_TRUE(payload.has_value());
218+
ASSERT_TRUE(payload.has_value());
231219
EXPECT_THAT(payload->error_message(),
232220
Eq("cannot align inputs to a common shape"));
233221
EXPECT_THAT(
@@ -246,7 +234,7 @@ TEST(ArollaEval, SimplePointwiseEval) {
246234
HasSubstr("shapes are not compatible: JaggedShape(1) "
247235
"vs JaggedShape(3)")));
248236
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
249-
EXPECT_TRUE(payload.has_value());
237+
ASSERT_TRUE(payload.has_value());
250238
EXPECT_THAT(payload->error_message(),
251239
Eq("cannot align inputs to a common shape"));
252240
EXPECT_THAT(
@@ -264,10 +252,6 @@ TEST(ArollaEval, SimplePointwiseEval) {
264252
status,
265253
StatusIs(absl::StatusCode::kInvalidArgument,
266254
HasSubstr("expected numerics, got y: DENSE_ARRAY_TEXT")));
267-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
268-
EXPECT_TRUE(payload.has_value());
269-
EXPECT_THAT(payload->error_message(),
270-
HasSubstr("expected numerics, got y: DENSE_ARRAY_TEXT"));
271255
}
272256
{
273257
// Arolla op eval error.
@@ -276,9 +260,6 @@ TEST(ArollaEval, SimplePointwiseEval) {
276260
auto status = SimplePointwiseEval("math.floordiv", {x, y}).status();
277261
EXPECT_THAT(status, StatusIs(absl::StatusCode::kInvalidArgument,
278262
HasSubstr("division by zero")));
279-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
280-
EXPECT_TRUE(payload.has_value());
281-
EXPECT_THAT(payload->error_message(), HasSubstr("division by zero"));
282263
}
283264
}
284265

@@ -412,13 +393,8 @@ TEST(ArollaEval, SimpleAggIntoEval) {
412393
// Scalar input error.
413394
DataSlice x = test::DataItem(1, schema::kObject);
414395
absl::Status status = SimpleAggIntoEval("math.sum", {x}).status();
415-
EXPECT_THAT(status,
416-
StatusIs(absl::StatusCode::kInvalidArgument,
417-
HasSubstr("expected rank(x) > 0")));
418-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
419-
EXPECT_TRUE(payload.has_value());
420-
EXPECT_THAT(payload->error_message(), Eq("math.sum: expected "
421-
"rank(x) > 0"));
396+
EXPECT_THAT(status, StatusIs(absl::StatusCode::kInvalidArgument,
397+
HasSubstr("expected rank(x) > 0")));
422398
}
423399
{
424400
// Mixed input error.
@@ -429,11 +405,6 @@ TEST(ArollaEval, SimpleAggIntoEval) {
429405
status,
430406
StatusIs(absl::StatusCode::kInvalidArgument,
431407
HasSubstr("DataSlice with mixed types is not supported")));
432-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
433-
EXPECT_TRUE(payload.has_value());
434-
EXPECT_THAT(payload->error_message(), Eq("math.sum: invalid inputs"));
435-
EXPECT_THAT(payload->cause().error_message(),
436-
HasSubstr("DataSlice with mixed types is not supported"));
437408
}
438409
{
439410
DataSlice x =
@@ -443,14 +414,6 @@ TEST(ArollaEval, SimpleAggIntoEval) {
443414
status,
444415
StatusIs(absl::StatusCode::kInvalidArgument,
445416
HasSubstr("expected numerics, got x: DENSE_ARRAY_TEXT")));
446-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
447-
EXPECT_TRUE(payload.has_value());
448-
EXPECT_THAT(payload->error_message(),
449-
Eq("math.max: successfully converted input DataSlice(s) to "
450-
"DenseArray(s) but failed to "
451-
"evaluate the Arolla operator"));
452-
EXPECT_THAT(payload->cause().error_message(),
453-
HasSubstr("expected numerics, got x: DENSE_ARRAY_TEXT"));
454417
}
455418
{
456419
// Eval with several inputs - default edge index.
@@ -573,12 +536,8 @@ TEST(ArollaEval, SimpleAggOverEval) {
573536
// Scalar input error.
574537
DataSlice x = test::DataItem(1, schema::kObject);
575538
absl::Status status = SimpleAggOverEval("math.sum", {x}).status();
576-
EXPECT_THAT(status,
577-
StatusIs(absl::StatusCode::kInvalidArgument,
578-
HasSubstr("expected rank(x) > 0")));
579-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
580-
EXPECT_TRUE(payload.has_value());
581-
EXPECT_THAT(payload->error_message(), Eq("math.sum: expected rank(x) > 0"));
539+
EXPECT_THAT(status, StatusIs(absl::StatusCode::kInvalidArgument,
540+
HasSubstr("expected rank(x) > 0")));
582541
}
583542
{
584543
// Mixed input error.
@@ -590,12 +549,6 @@ TEST(ArollaEval, SimpleAggOverEval) {
590549
status,
591550
StatusIs(absl::StatusCode::kInvalidArgument,
592551
HasSubstr("DataSlice with mixed types is not supported")));
593-
std::optional<internal::Error> payload = internal::GetErrorPayload(status);
594-
EXPECT_TRUE(payload.has_value());
595-
EXPECT_THAT(payload->error_message(),
596-
Eq("array.inverse_mapping: invalid inputs"));
597-
EXPECT_THAT(payload->cause().error_message(),
598-
HasSubstr("DataSlice with mixed types is not supported"));
599552
}
600553
{
601554
// Eval with several inputs - default edge index.

‎py/koladata/operators/tests/simple_agg_arolla_op_error_test.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,16 @@ def test_mixed_types_input_error(self):
3838
with self.assertRaisesRegex(
3939
exceptions.KodaError,
4040
re.escape(
41-
"""math.max: invalid inputs
42-
43-
The cause is: DataSlice with mixed types is not supported: DataSlice([1, 2.0], schema: OBJECT, shape: JaggedShape(2))"""
41+
'kd.math.agg_max: DataSlice with mixed types is not supported:'
42+
' DataSlice([1, 2.0], schema: OBJECT, shape: JaggedShape(2))'
4443
),
4544
):
4645
expr_eval.eval(kde.agg_max(x))
4746

4847
def test_expect_rank_error(self):
4948
with self.assertRaisesRegex(
5049
exceptions.KodaError,
51-
re.escape("""math.max: expected rank(x) > 0"""),
50+
re.escape('kd.math.agg_max: expected rank(x) > 0'),
5251
):
5352
expr_eval.eval(kde.agg_max(ds(1)))
5453

‎py/koladata/operators/tests/slices_sort_test.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def test_data_item(self):
145145
exceptions.KodaError,
146146
# TODO: b/374841918 - For lambdas we only report underlying operator
147147
# names.
148-
re.escape('array.ordinal_rank: expected rank(x) > 0'),
148+
re.escape('kd.slices.ordinal_rank: expected rank(x) > 0'),
149149
):
150150
expr_eval.eval(kde.slices.sort(ds(0)))
151151

0 commit comments

Comments
 (0)