[ 
https://issues.apache.org/jira/browse/ARROW-1884?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16282514#comment-16282514
 ] 

ASF GitHub Bot commented on ARROW-1884:
---------------------------------------

wesm closed pull request #1400: ARROW-1884: [C++] Exclude integration test JSON 
reader/writer classes from public API
URL: https://github.com/apache/arrow/pull/1400
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/cpp/src/arrow/ipc/ipc-json-test.cc 
b/cpp/src/arrow/ipc/ipc-json-test.cc
index e496826f9..12fa4bf3e 100644
--- a/cpp/src/arrow/ipc/ipc-json-test.cc
+++ b/cpp/src/arrow/ipc/ipc-json-test.cc
@@ -39,6 +39,7 @@
 
 namespace arrow {
 namespace ipc {
+namespace internal {
 namespace json {
 
 void TestSchemaRoundTrip(const Schema& schema) {
@@ -46,7 +47,7 @@ void TestSchemaRoundTrip(const Schema& schema) {
   rj::Writer<rj::StringBuffer> writer(sb);
 
   writer.StartObject();
-  ASSERT_OK(internal::WriteSchema(schema, &writer));
+  ASSERT_OK(WriteSchema(schema, &writer));
   writer.EndObject();
 
   std::string json_schema = sb.GetString();
@@ -55,7 +56,7 @@ void TestSchemaRoundTrip(const Schema& schema) {
   d.Parse(json_schema);
 
   std::shared_ptr<Schema> out;
-  if (!internal::ReadSchema(d, default_memory_pool(), &out).ok()) {
+  if (!ReadSchema(d, default_memory_pool(), &out).ok()) {
     FAIL() << "Unable to read JSON schema: " << json_schema;
   }
 
@@ -70,7 +71,7 @@ void TestArrayRoundTrip(const Array& array) {
   rj::StringBuffer sb;
   rj::Writer<rj::StringBuffer> writer(sb);
 
-  ASSERT_OK(internal::WriteArray(name, array, &writer));
+  ASSERT_OK(WriteArray(name, array, &writer));
 
   std::string array_as_json = sb.GetString();
 
@@ -82,7 +83,7 @@ void TestArrayRoundTrip(const Array& array) {
   }
 
   std::shared_ptr<Array> out;
-  ASSERT_OK(internal::ReadArray(default_memory_pool(), d, array.type(), &out));
+  ASSERT_OK(ReadArray(default_memory_pool(), d, array.type(), &out));
 
   // std::cout << array_as_json << std::endl;
   CompareArraysDetailed(0, *out, array);
@@ -415,5 +416,6 @@ TEST_P(TestJsonRoundTrip, RoundTrip) {
 INSTANTIATE_TEST_CASE_P(TestJsonRoundTrip, TestJsonRoundTrip, BATCH_CASES());
 
 }  // namespace json
+}  // namespace internal
 }  // namespace ipc
 }  // namespace arrow
diff --git a/cpp/src/arrow/ipc/json-integration-test.cc 
b/cpp/src/arrow/ipc/json-integration-test.cc
index f362d9701..37778fa25 100644
--- a/cpp/src/arrow/ipc/json-integration-test.cc
+++ b/cpp/src/arrow/ipc/json-integration-test.cc
@@ -50,8 +50,7 @@ DEFINE_bool(verbose, true, "Verbose output");
 namespace fs = boost::filesystem;
 
 namespace arrow {
-
-class Buffer;
+namespace ipc {
 
 bool file_exists(const char* path) {
   std::ifstream handle(path);
@@ -73,16 +72,15 @@ static Status ConvertJsonToArrow(const std::string& 
json_path,
   std::shared_ptr<Buffer> json_buffer;
   RETURN_NOT_OK(in_file->Read(file_size, &json_buffer));
 
-  std::unique_ptr<ipc::JsonReader> reader;
-  RETURN_NOT_OK(ipc::JsonReader::Open(json_buffer, &reader));
+  std::unique_ptr<internal::json::JsonReader> reader;
+  RETURN_NOT_OK(internal::json::JsonReader::Open(json_buffer, &reader));
 
   if (FLAGS_verbose) {
     std::cout << "Found schema: " << reader->schema()->ToString() << std::endl;
   }
 
-  std::shared_ptr<ipc::RecordBatchWriter> writer;
-  RETURN_NOT_OK(
-      ipc::RecordBatchFileWriter::Open(out_file.get(), reader->schema(), 
&writer));
+  std::shared_ptr<RecordBatchWriter> writer;
+  RETURN_NOT_OK(RecordBatchFileWriter::Open(out_file.get(), reader->schema(), 
&writer));
 
   for (int i = 0; i < reader->num_record_batches(); ++i) {
     std::shared_ptr<RecordBatch> batch;
@@ -101,15 +99,15 @@ static Status ConvertArrowToJson(const std::string& 
arrow_path,
   RETURN_NOT_OK(io::ReadableFile::Open(arrow_path, &in_file));
   RETURN_NOT_OK(io::FileOutputStream::Open(json_path, &out_file));
 
-  std::shared_ptr<ipc::RecordBatchFileReader> reader;
-  RETURN_NOT_OK(ipc::RecordBatchFileReader::Open(in_file.get(), &reader));
+  std::shared_ptr<RecordBatchFileReader> reader;
+  RETURN_NOT_OK(RecordBatchFileReader::Open(in_file.get(), &reader));
 
   if (FLAGS_verbose) {
     std::cout << "Found schema: " << reader->schema()->ToString() << std::endl;
   }
 
-  std::unique_ptr<ipc::JsonWriter> writer;
-  RETURN_NOT_OK(ipc::JsonWriter::Open(reader->schema(), &writer));
+  std::unique_ptr<internal::json::JsonWriter> writer;
+  RETURN_NOT_OK(internal::json::JsonWriter::Open(reader->schema(), &writer));
 
   for (int i = 0; i < reader->num_record_batches(); ++i) {
     std::shared_ptr<RecordBatch> batch;
@@ -134,15 +132,15 @@ static Status ValidateArrowVsJson(const std::string& 
arrow_path,
   std::shared_ptr<Buffer> json_buffer;
   RETURN_NOT_OK(json_file->Read(file_size, &json_buffer));
 
-  std::unique_ptr<ipc::JsonReader> json_reader;
-  RETURN_NOT_OK(ipc::JsonReader::Open(json_buffer, &json_reader));
+  std::unique_ptr<internal::json::JsonReader> json_reader;
+  RETURN_NOT_OK(internal::json::JsonReader::Open(json_buffer, &json_reader));
 
   // Construct Arrow reader
   std::shared_ptr<io::ReadableFile> arrow_file;
   RETURN_NOT_OK(io::ReadableFile::Open(arrow_path, &arrow_file));
 
-  std::shared_ptr<ipc::RecordBatchFileReader> arrow_reader;
-  RETURN_NOT_OK(ipc::RecordBatchFileReader::Open(arrow_file.get(), 
&arrow_reader));
+  std::shared_ptr<RecordBatchFileReader> arrow_reader;
+  RETURN_NOT_OK(RecordBatchFileReader::Open(arrow_file.get(), &arrow_reader));
 
   auto json_schema = json_reader->schema();
   auto arrow_schema = arrow_reader->schema();
@@ -399,6 +397,7 @@ TEST_F(TestJSONIntegration, ErrorStates) {
   ASSERT_RAISES(Invalid, RunCommand(json_path, "", "VALIDATE"));
 }
 
+}  // namespace ipc
 }  // namespace arrow
 
 int main(int argc, char** argv) {
@@ -407,7 +406,7 @@ int main(int argc, char** argv) {
   int ret = 0;
 
   if (FLAGS_integration) {
-    arrow::Status result = arrow::RunCommand(FLAGS_json, FLAGS_arrow, 
FLAGS_mode);
+    arrow::Status result = arrow::ipc::RunCommand(FLAGS_json, FLAGS_arrow, 
FLAGS_mode);
     if (!result.ok()) {
       std::cout << "Error message: " << result.ToString() << std::endl;
       ret = 1;
diff --git a/cpp/src/arrow/ipc/json-internal.cc 
b/cpp/src/arrow/ipc/json-internal.cc
index 11a6956eb..4088a8f20 100644
--- a/cpp/src/arrow/ipc/json-internal.cc
+++ b/cpp/src/arrow/ipc/json-internal.cc
@@ -40,8 +40,11 @@
 
 namespace arrow {
 namespace ipc {
-namespace json {
 namespace internal {
+namespace json {
+
+using ::arrow::ipc::DictionaryMemo;
+using ::arrow::ipc::DictionaryTypeMap;
 
 static std::string GetFloatingPrecisionName(FloatingPoint::Precision 
precision) {
   switch (precision) {
@@ -1463,7 +1466,7 @@ Status ReadArray(MemoryPool* pool, const rj::Value& 
json_array, const Schema& sc
   return ReadArray(pool, json_array, result->type(), array);
 }
 
-}  // namespace internal
 }  // namespace json
+}  // namespace internal
 }  // namespace ipc
 }  // namespace arrow
diff --git a/cpp/src/arrow/ipc/json-internal.h 
b/cpp/src/arrow/ipc/json-internal.h
index 506fe6829..92afc1444 100644
--- a/cpp/src/arrow/ipc/json-internal.h
+++ b/cpp/src/arrow/ipc/json-internal.h
@@ -92,8 +92,8 @@ using RjObject = rj::Value::ConstObject;
 
 namespace arrow {
 namespace ipc {
-namespace json {
 namespace internal {
+namespace json {
 
 Status WriteSchema(const Schema& schema, RjWriter* writer);
 Status WriteRecordBatch(const RecordBatch& batch, RjWriter* writer);
@@ -111,8 +111,8 @@ Status ReadArray(MemoryPool* pool, const rj::Value& 
json_obj,
 Status ReadArray(MemoryPool* pool, const rj::Value& json_obj, const Schema& 
schema,
                  std::shared_ptr<Array>* array);
 
-}  // namespace internal
 }  // namespace json
+}  // namespace internal
 }  // namespace ipc
 }  // namespace arrow
 
diff --git a/cpp/src/arrow/ipc/json.cc b/cpp/src/arrow/ipc/json.cc
index ea2947d5d..394563c53 100644
--- a/cpp/src/arrow/ipc/json.cc
+++ b/cpp/src/arrow/ipc/json.cc
@@ -33,6 +33,8 @@ using std::size_t;
 
 namespace arrow {
 namespace ipc {
+namespace internal {
+namespace json {
 
 // ----------------------------------------------------------------------
 // Writer implementation
@@ -45,7 +47,7 @@ class JsonWriter::JsonWriterImpl {
 
   Status Start() {
     writer_->StartObject();
-    RETURN_NOT_OK(json::internal::WriteSchema(*schema_, writer_.get()));
+    RETURN_NOT_OK(json::WriteSchema(*schema_, writer_.get()));
 
     // Record batches
     writer_->Key("batches");
@@ -63,7 +65,7 @@ class JsonWriter::JsonWriterImpl {
 
   Status WriteRecordBatch(const RecordBatch& batch) {
     DCHECK_EQ(batch.num_columns(), schema_->num_fields());
-    return json::internal::WriteRecordBatch(batch, writer_.get());
+    return json::WriteRecordBatch(batch, writer_.get());
   }
 
  private:
@@ -106,7 +108,7 @@ class JsonReader::JsonReaderImpl {
       return Status::IOError("JSON parsing failed");
     }
 
-    RETURN_NOT_OK(json::internal::ReadSchema(doc_, pool_, &schema_));
+    RETURN_NOT_OK(json::ReadSchema(doc_, pool_, &schema_));
 
     auto it = doc_.FindMember("batches");
     RETURN_NOT_ARRAY("batches", it, doc_);
@@ -120,8 +122,7 @@ class JsonReader::JsonReaderImpl {
     DCHECK_LT(i, static_cast<int>(record_batches_->GetArray().Size()))
         << "i out of bounds";
 
-    return json::internal::ReadRecordBatch(record_batches_->GetArray()[i], 
schema_, pool_,
-                                           batch);
+    return json::ReadRecordBatch(record_batches_->GetArray()[i], schema_, 
pool_, batch);
   }
 
   std::shared_ptr<Schema> schema() const { return schema_; }
@@ -164,5 +165,7 @@ Status JsonReader::ReadRecordBatch(int i, 
std::shared_ptr<RecordBatch>* batch) c
   return impl_->ReadRecordBatch(i, batch);
 }
 
+}  // namespace json
+}  // namespace internal
 }  // namespace ipc
 }  // namespace arrow
diff --git a/cpp/src/arrow/ipc/json.h b/cpp/src/arrow/ipc/json.h
index 51f30f0c1..674c3745e 100644
--- a/cpp/src/arrow/ipc/json.h
+++ b/cpp/src/arrow/ipc/json.h
@@ -34,12 +34,14 @@ class RecordBatch;
 class Schema;
 
 namespace ipc {
+namespace internal {
+namespace json {
 
 /// \class JsonWriter
 /// \brief Write the JSON representation of an Arrow record batch file or 
stream
 ///
 /// This is used for integration testing
-class ARROW_EXPORT JsonWriter {
+class JsonWriter {
  public:
   ~JsonWriter();
 
@@ -72,7 +74,7 @@ class ARROW_EXPORT JsonWriter {
 /// \brief Read the JSON representation of an Arrow record batch file or stream
 ///
 /// This is used for integration testing
-class ARROW_EXPORT JsonReader {
+class JsonReader {
  public:
   ~JsonReader();
 
@@ -113,6 +115,8 @@ class ARROW_EXPORT JsonReader {
   std::unique_ptr<JsonReaderImpl> impl_;
 };
 
+}  // namespace json
+}  // namespace internal
 }  // namespace ipc
 }  // namespace arrow
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> [C++] Make JsonReader/JsonWriter classes internal APIs
> ------------------------------------------------------
>
>                 Key: ARROW-1884
>                 URL: https://issues.apache.org/jira/browse/ARROW-1884
>             Project: Apache Arrow
>          Issue Type: Improvement
>          Components: C++
>            Reporter: Wes McKinney
>            Assignee: Wes McKinney
>              Labels: pull-request-available
>             Fix For: 0.8.0
>
>
> These are exposed in the public API in {{arrow::ipc}}, and could possibly 
> mislead users: http://arrow.apache.org/docs/cpp/namespacearrow_1_1ipc.html



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to