You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/05/24 13:51:39 UTC
[GitHub] [arrow] lidavidm opened a new pull request #10386: ARROW-12859: [C++] Add DatumFromJSON for testing
lidavidm opened a new pull request #10386:
URL: https://github.com/apache/arrow/pull/10386
```
DatumFromJSON(int64(), "null") = Datum(Int64Scalar(null))
DatumFromJSON(int64(), "5") = Datum(Int64Scalar(5))
DatumFromJSON(int64(), "[5]") = Datum(Int64Array([5]))
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10386: ARROW-12859: [C++] Add ScalarFromJSON for testing
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10386:
URL: https://github.com/apache/arrow/pull/10386#issuecomment-852272551
Can you explain the `Scalar::CastTo` suggestion? I think I'm missing something.
Also, I notice this isn't actually tested? Perhaps add a test alongside those for `ArrayFromJSON`. And ensure that a non-1 length array would return an error?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #10386: ARROW-12859: [C++] Add ScalarFromJSON for testing
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #10386:
URL: https://github.com/apache/arrow/pull/10386#issuecomment-852463069
For CastTo: I think it was actually the Cast kernel (string->type cast).
I added some basic tests. I don't think there's a good way to hit the DCHECK because that would imply a JSON value converter appended two array values for a single JSON value.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #10386: ARROW-12859: [C++] Add ScalarFromJSON for testing
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #10386:
URL: https://github.com/apache/arrow/pull/10386#discussion_r638851046
##########
File path: cpp/src/arrow/testing/gtest_util.cc
##########
@@ -396,6 +438,13 @@ std::shared_ptr<RecordBatch> RecordBatchFromJSON(const std::shared_ptr<Schema>&
return *RecordBatch::FromStructArray(struct_array);
}
+std::shared_ptr<Scalar> ScalarFromJSON(const std::shared_ptr<DataType>& type,
+ util::string_view json) {
+ std::shared_ptr<Scalar> out;
+ ABORT_NOT_OK(ipc::internal::json::ScalarFromJSON(type, json, &out));
Review comment:
This approach might be worthwhile for consistency's sake, but we could also use `Scalar::CastTo`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #10386: ARROW-12859: [C++] Add ScalarFromJSON for testing
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #10386:
URL: https://github.com/apache/arrow/pull/10386
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10386: ARROW-12859: [C++] Add DatumFromJSON for testing
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10386:
URL: https://github.com/apache/arrow/pull/10386#issuecomment-847075103
https://issues.apache.org/jira/browse/ARROW-12859
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #10386: ARROW-12859: [C++] Add ScalarFromJSON for testing
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #10386:
URL: https://github.com/apache/arrow/pull/10386#issuecomment-852265682
> Is there anything left to do here?
I don't think so unless we really want to use Scalar::CastTo here.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10386: ARROW-12859: [C++] Add ScalarFromJSON for testing
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10386:
URL: https://github.com/apache/arrow/pull/10386#issuecomment-852252521
Is there anything left to do here?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org