ARROW-15611: [C++] Migrate arrow::ipc::internal::json::ArrayFromJSON to Result<> #12713
Conversation
|
|
cpp/src/arrow/python/gdb.cc
Outdated
| @@ -29,6 +29,7 @@ | |||
| #include "arrow/scalar.h" | |||
| #include "arrow/table.h" | |||
| #include "arrow/type.h" | |||
| #include "arrow/testing/gtest_util.h" | |||
There was a problem hiding this comment.
I'm not sure we want this linked to googletest, we should stick to the old version.
AlvinJ15
force-pushed
the
ARROW-15611#Migrate_arrow_ipc_internal_json_ArrayFromJSON_to_Result
branch
3 times, most recently
from
4de1c69
to
749327a
Compare
cpp/src/arrow/python/gdb.h
Outdated
| @@ -23,7 +23,7 @@ namespace arrow { | |||
| namespace gdb { | |||
|
|
|||
| ARROW_PYTHON_EXPORT | |||
| void TestSession(); | |||
| Status TestSession(); | |||
There was a problem hiding this comment.
We need to update the binding signature on the Cython side.
|
Er, whoops. Thanks for the changes, there's just a minor fix we should make. I think the S3 test issues are due to ARROW-16043, and we need to figure out why the GDB tests fail. |
cpp/src/arrow/python/gdb.cc
Outdated
| // We define local variables for all types for which we want to test | ||
| // pretty-printing. | ||
| // Then, at the end of this function, we trap to the debugger, so that | ||
| // test instrumentation can print values from this frame by interacting | ||
| // with the debugger. | ||
| // The test instrumentation is in pyarrow/tests/test_gdb.py | ||
| // The test instrumentation is in pyarrow/tests/test_gdb.pytest_gdb.py |
There was a problem hiding this comment.
I think that change is not deliberate, is it?
cpp/src/arrow/python/gdb.cc
Outdated
| Result<std::shared_ptr<Array>> SliceArrayFromJSON(const std::shared_ptr<DataType>& ty, | ||
| util::string_view json, | ||
| int64_t offset = 0, | ||
| int64_t length = -1) { | ||
| ARROW_ASSIGN_OR_RAISE(auto array, ArrayFromJSON(ty, json)); |
There was a problem hiding this comment.
I don't think this change is necessary, instead, you could just have:
| Result<std::shared_ptr<Array>> SliceArrayFromJSON(const std::shared_ptr<DataType>& ty, | |
| util::string_view json, | |
| int64_t offset = 0, | |
| int64_t length = -1) { | |
| ARROW_ASSIGN_OR_RAISE(auto array, ArrayFromJSON(ty, json)); | |
| std::shared_ptr<Array> SliceArrayFromJSON(const std::shared_ptr<DataType>& ty, | |
| util::string_view json, int64_t offset = 0, | |
| int64_t length = -1) { | |
| auto array = *ArrayFromJSON(ty, json); |
cpp/src/arrow/python/gdb.cc
Outdated
| @@ -98,13 +96,13 @@ std::shared_ptr<Array> SliceArrayFromJSON(const std::shared_ptr<DataType>& ty, | |||
|
|
|||
| } // namespace | |||
|
|
|||
| void TestSession() { | |||
| Status TestSession() { | |||
There was a problem hiding this comment.
Similarly, this change doesn't seem necessary.
AlvinJ15
force-pushed
the
ARROW-15611#Migrate_arrow_ipc_internal_json_ArrayFromJSON_to_Result
branch
from
749327a
to
629f9fb
Compare
|
Benchmark runs are scheduled for baseline = 556ee23 and contender = 77e2a11. 77e2a11 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Migrate arrow::ipc::internal::json::ArrayFromJSON to Result<>