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 2022/07/19 09:25:35 UTC
[GitHub] [arrow] ZhangChaoming opened a new pull request, #13648: MINOR: [C++] Row wise conversion example fix
ZhangChaoming opened a new pull request, #13648:
URL: https://github.com/apache/arrow/pull/13648
This example should print the expected rows.
--
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] pitrou commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1188971628
Can you explain why it should print the expected rows?
--
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 #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1190710374
Benchmark runs are scheduled for baseline = 6e3f26af658bfca602e711ea326f1985b62bca1d and contender = c445243a1455a7e06c7bed95596499e348056bac. c445243a1455a7e06c7bed95596499e348056bac is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a68e84d185a04ab6a774739da12824c0...e236dc0baca944c39aba0b10373ee78d/)
[Failed :arrow_down:0.31% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/cfe93d1cb7e842fa87e88df0ab4067e9...6ca53824b9bb4138b82381c91d3f5287/)
[Failed] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/cbb6f4c75a70469692b95e3047f6fc03...78939a04be7e4e6b9c0047ad92848c03/)
[Finished :arrow_down:0.68% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6705564e8ecf475889623c02225a3203...e39359f410b843cf967af5fa572cbc08/)
Buildkite builds:
[Failed] [`c445243a` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1150)
[Failed] [`c445243a` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1155)
[Failed] [`c445243a` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1142)
[Finished] [`c445243a` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1159)
[Failed] [`6e3f26af` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1149)
[Failed] [`6e3f26af` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1154)
[Failed] [`6e3f26af` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1141)
[Finished] [`6e3f26af` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1158)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] ZhangChaoming commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
ZhangChaoming commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189064902
@pitrou DONE
--
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] lidavidm commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1188991890
Hmm, it seems the example flips the names of `rows` and `expected_rows` - `rows` is hardcoded data and `expected_rows` is the runtime 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189010432
Hmm... can we instead avoid flipping `rows` and `expected_rows` to make the example less confusing?
--
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] pitrou commented on a diff in pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #13648:
URL: https://github.com/apache/arrow/pull/13648#discussion_r924819106
##########
cpp/examples/arrow/row_wise_conversion_example.cc:
##########
@@ -172,17 +172,19 @@ arrow::Result<std::vector<data_row>> ColumnarTableToVector(
return rows;
}
+
arrow::Status RunRowConversion() {
- std::vector<data_row> rows = {
+ std::vector<data_row> original_rows = {
{1, 1, {10.0}}, {2, 3, {11.0, 12.0, 13.0}}, {3, 2, {15.0, 25.0}}};
std::shared_ptr<arrow::Table> table;
- std::vector<data_row> expected_rows;
+ std::vector<data_row> converted_rows
Review Comment:
```suggestion
std::vector<data_row> converted_rows,
```
--
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] ZhangChaoming commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
ZhangChaoming commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189082984
@lidavidm Could you please help review this?
--
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] pitrou merged pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou merged PR #13648:
URL: https://github.com/apache/arrow/pull/13648
--
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] pitrou commented on a diff in pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #13648:
URL: https://github.com/apache/arrow/pull/13648#discussion_r924819467
##########
cpp/examples/arrow/row_wise_conversion_example.cc:
##########
@@ -172,17 +172,19 @@ arrow::Result<std::vector<data_row>> ColumnarTableToVector(
return rows;
}
+
arrow::Status RunRowConversion() {
- std::vector<data_row> rows = {
+ std::vector<data_row> original_rows = {
{1, 1, {10.0}}, {2, 3, {11.0, 12.0, 13.0}}, {3, 2, {15.0, 25.0}}};
std::shared_ptr<arrow::Table> table;
- std::vector<data_row> expected_rows;
+ std::vector<data_row> converted_rows,
Review Comment:
```suggestion
std::vector<data_row> converted_rows;
```
--
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] pitrou commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189475275
Thanks @ZhangChaoming !
--
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] lidavidm commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189028767
Swap the variable names.
--
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] pitrou commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189049433
Looks like there is a merge conflict with git master now, could you solve it @ZhangChaoming ?
--
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] ZhangChaoming commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
ZhangChaoming commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189002520
@lidavidm Yep. Some check were not successful, but I think my changes is safe, could you please help merge it?
--
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] ZhangChaoming commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
ZhangChaoming commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189027451
> Hmm... can we instead avoid flipping `rows` and `expected_rows` to make the example less confusing?
I did not get it, you mean just use one variable `rows` ?
--
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] pitrou commented on pull request #13648: MINOR: [C++] Row wise conversion example fix
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #13648:
URL: https://github.com/apache/arrow/pull/13648#issuecomment-1189115074
@ZhangChaoming Thanks, can you look at the CI failures now? :-)
--
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