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/11/10 16:45:16 UTC

[GitHub] [arrow] dhruv9vats opened a new pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

dhruv9vats opened a new pull request #11663:
URL: https://github.com/apache/arrow/pull/11663


   Update unconditional exposed `std::cout`s in unittests to use ARROW_SCOPED_TRACE or SCOPED_TRACE macros as appropriate.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] westonpace commented on a change in pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#discussion_r750139221



##########
File path: cpp/src/arrow/compute/exec/hash_join_node_test.cc
##########
@@ -1061,16 +1061,18 @@ TEST(HashJoin, Random) {
 
     // Print test case parameters
     // print num_rows, batch_size, join_type, join_cmp
-    std::cout << join_type_name << " " << key_cmp_str << " ";
-    key_types.Print();
-    std::cout << " payload_l: ";
-    payload_types[0].Print();
-    std::cout << " payload_r: ";
-    payload_types[1].Print();
-    std::cout << " num_rows_l = " << num_rows_l << " num_rows_r = " << num_rows_r
-              << " batch size = " << batch_size
-              << " parallel = " << (parallel ? "true" : "false");
-    std::cout << std::endl;
+    // std::cout << join_type_name << " " << key_cmp_str << " ";
+    // key_types.Print();
+    // std::cout << " payload_l: ";
+    // payload_types[0].Print();
+    // std::cout << " payload_r: ";
+    // payload_types[1].Print();
+    // std::cout << " num_rows_l = " << num_rows_l << " num_rows_r = " << num_rows_r
+    //           << " batch size = " << batch_size
+    //           << " parallel = " << (parallel ? "true" : "false");
+    // std::cout << std::endl;

Review comment:
       Go ahead and just delete these lines entirely.  No need to leave the commented out code.

##########
File path: cpp/src/gandiva/tests/date_time_test.cc
##########
@@ -452,7 +452,8 @@ TEST_F(TestProjector, TestTimestampDiffMonth) {
   std::shared_ptr<Projector> projector;
   auto status =
       Projector::Make(schema, {diff_months_expr}, TestConfiguration(), &projector);
-  std::cout << status.message();
+  // std::cout << status.message();
+  SCOPED_TRACE(status.message());

Review comment:
       Just delete this, no need for scoped_trace here.  If status is not `ok` then the test will fail and it will be obvious where it failed.  Status is not a "test parameter"
   
   Same with the next two occurrences.

##########
File path: cpp/src/parquet/column_writer_test.cc
##########
@@ -254,7 +254,8 @@ class TestPrimitiveWriter : public PrimitiveTypedTest<TestType> {
     for (size_t i = 0; i < this->values_.size(); i++) {
       if (comparator->Compare(this->values_[i], this->values_out_[i]) ||
           comparator->Compare(this->values_out_[i], this->values_[i])) {
-        std::cout << "Failed at " << i << std::endl;
+        // std::cout << "Failed at " << i << std::endl;
+        ARROW_SCOPED_TRACE("Failed at ", i);

Review comment:
       The message should be more like `ARROW_SCOPED_TRACE("i=",i);`.  The trace should tell use the parameter values, it doesn't need to include "failed at" as that will be implied (the trace info is only shown on a failure).




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] westonpace closed pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
westonpace closed pull request #11663:
URL: https://github.com/apache/arrow/pull/11663


   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-974584498


   Benchmark runs are scheduled for baseline = 5b68c8e8584b2e6261627f4996b1ffbbb873a0bc and contender = 2155d46494a340111453a9696e304dd5eca83919. 2155d46494a340111453a9696e304dd5eca83919 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/1ae26fb012b74fccb79d30f263f21e90...f72531a5888c40a899dca1a6c70eecc8/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b3e7f7fcc90a4c3b9a9d97103ff34bde...62dbd59d5c9b4bc4abbe54cae70a24ef/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/21859bb388d842acab6a25d20965d366...a0a44e1a282e41c8be4ea5f06080f209/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] dhruv9vats edited a comment on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
dhruv9vats edited a comment on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-965542750


   Haven't altered conditional print statements, like for example, at [json_integration_test.cc#L86](https://github.com/apache/arrow/blob/master/cpp/src/arrow/testing/json_integration_test.cc#L86) like:
   ```cpp
   if (FLAGS_verbose) {
       std::cout << "Found schema:\n"
                 << reader->schema()->ToString(/* show_metadata = */ true) << std::endl;
     }
   ```
   
   @westonpace 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] dhruv9vats commented on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
dhruv9vats commented on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-965542750


   Haven't altered conditional print statements, like for example, at [json_integration_test.cc#L86](https://github.com/apache/arrow/blob/master/cpp/src/arrow/testing/json_integration_test.cc#L86) like:
   ```cpp
   if (FLAGS_verbose) {
       std::cout << "Found schema:\n"
                 << reader->schema()->ToString(/* show_metadata = */ true) << std::endl;
     }
   ```


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-974584498


   Benchmark runs are scheduled for baseline = 5b68c8e8584b2e6261627f4996b1ffbbb873a0bc and contender = 2155d46494a340111453a9696e304dd5eca83919. 2155d46494a340111453a9696e304dd5eca83919 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/1ae26fb012b74fccb79d30f263f21e90...f72531a5888c40a899dca1a6c70eecc8/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b3e7f7fcc90a4c3b9a9d97103ff34bde...62dbd59d5c9b4bc4abbe54cae70a24ef/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/21859bb388d842acab6a25d20965d366...a0a44e1a282e41c8be4ea5f06080f209/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot commented on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-974584498


   Benchmark runs are scheduled for baseline = 5b68c8e8584b2e6261627f4996b1ffbbb873a0bc and contender = 2155d46494a340111453a9696e304dd5eca83919. 2155d46494a340111453a9696e304dd5eca83919 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/1ae26fb012b74fccb79d30f263f21e90...f72531a5888c40a899dca1a6c70eecc8/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b3e7f7fcc90a4c3b9a9d97103ff34bde...62dbd59d5c9b4bc4abbe54cae70a24ef/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/21859bb388d842acab6a25d20965d366...a0a44e1a282e41c8be4ea5f06080f209/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] westonpace commented on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
westonpace commented on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-970859669


   @dhruv9vats Yes please :)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] dhruv9vats commented on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
dhruv9vats commented on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-970214952


   Should I also remove [these](https://github.com/apache/arrow/blob/f1fcc4f2bfe5b115bda96fa547da4c0b4328685c/cpp/src/arrow/util/future_test.cc#L1317) commented print statements, seemingly used for rule-of-5 debugging? 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-965533506






-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-974584498


   Benchmark runs are scheduled for baseline = 5b68c8e8584b2e6261627f4996b1ffbbb873a0bc and contender = 2155d46494a340111453a9696e304dd5eca83919. 2155d46494a340111453a9696e304dd5eca83919 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/1ae26fb012b74fccb79d30f263f21e90...f72531a5888c40a899dca1a6c70eecc8/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b3e7f7fcc90a4c3b9a9d97103ff34bde...62dbd59d5c9b4bc4abbe54cae70a24ef/)
   [Finished :arrow_down:0.18% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/21859bb388d842acab6a25d20965d366...a0a44e1a282e41c8be4ea5f06080f209/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #11663: ARROW-14641: [C++][Compute] Reduce print statements from unit tests

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#issuecomment-974584498


   Benchmark runs are scheduled for baseline = 5b68c8e8584b2e6261627f4996b1ffbbb873a0bc and contender = 2155d46494a340111453a9696e304dd5eca83919. 2155d46494a340111453a9696e304dd5eca83919 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/1ae26fb012b74fccb79d30f263f21e90...f72531a5888c40a899dca1a6c70eecc8/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b3e7f7fcc90a4c3b9a9d97103ff34bde...62dbd59d5c9b4bc4abbe54cae70a24ef/)
   [Finished :arrow_down:0.18% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/21859bb388d842acab6a25d20965d366...a0a44e1a282e41c8be4ea5f06080f209/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org