pitrou commented on a change in pull request #12713:
URL: https://github.com/apache/arrow/pull/12713#discussion_r836492655
##########
File path: cpp/src/arrow/python/gdb.cc
##########
@@ -98,13 +96,13 @@ std::shared_ptr<Array> SliceArrayFromJSON(const
std::shared_ptr<DataType>& ty,
} // namespace
-void TestSession() {
+Status TestSession() {
Review comment:
Similarly, this change doesn't seem necessary.
##########
File path: cpp/src/arrow/python/gdb.cc
##########
@@ -98,13 +96,13 @@ std::shared_ptr<Array> SliceArrayFromJSON(const
std::shared_ptr<DataType>& ty,
} // namespace
-void TestSession() {
+Status TestSession() {
// 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
Review comment:
I think that change is not deliberate, is it?
##########
File path: cpp/src/arrow/python/gdb.cc
##########
@@ -82,13 +82,11 @@ class UuidType : public ExtensionType {
std::string Serialize() const override { return "uuid-serialized"; }
};
-// TODO migrate arrow::ipc::internal::json::ArrayFromJSON to Result<>?
-
-std::shared_ptr<Array> SliceArrayFromJSON(const std::shared_ptr<DataType>& ty,
- util::string_view json, int64_t
offset = 0,
- int64_t length = -1) {
- std::shared_ptr<Array> array;
- ARROW_CHECK_OK(ArrayFromJSON(ty, json, &array));
+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));
Review comment:
I don't think this change is necessary, instead, you could just have:
```suggestion
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);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
