You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2026/04/11 14:23:49 UTC
[PR] GH-40742: [R] fix max_rows_per_group must be a positive number [arrow]
thisisnic opened a new pull request, #49709:
URL: https://github.com/apache/arrow/pull/49709
### Rationale for this change
Error message due to faulty checking
### What changes are included in this PR?
Change checking circumstances
### Are these changes tested?
Yes
### Are there any user-facing changes?
No
### AI usage
Used Claude to generate this - will double check before marking ready for review
--
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
Re: [PR] GH-40742: [R] fix max_rows_per_group must be a positive number [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49709:
URL: https://github.com/apache/arrow/pull/49709#issuecomment-4229580607
:warning: GitHub issue #40742 **has been automatically assigned in GitHub** to PR creator.
--
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
Re: [PR] GH-40742: [R] fix max_rows_per_group must be a positive number [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #49709:
URL: https://github.com/apache/arrow/pull/49709#issuecomment-4380378999
After merging your PR, Conbench analyzed the 0 benchmarking runs that have been run so far on merge-commit 9a0afd9e9545444b492237cc9db7eb4549ce09f5.
None of the specified runs were found on the Conbench server.
The [full Conbench report](https://github.com/apache/arrow/runs/74437874718) has more details.
--
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
Re: [PR] GH-40742: [R] fix max_rows_per_group must be a positive number [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #49709:
URL: https://github.com/apache/arrow/pull/49709
--
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
Re: [PR] GH-40742: [R] fix max_rows_per_group must be a positive number [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #49709:
URL: https://github.com/apache/arrow/pull/49709#issuecomment-4455666365
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 9a0afd9e9545444b492237cc9db7eb4549ce09f5.
There was 1 benchmark result indicating a performance regression:
- Commit Run on `amd64-m5-4xlarge-linux` at [2026-05-14 22:04:15Z](https://conbench.arrow-dev.org/compare/runs/9d26564356824807b15773ee2a4a7c8b...3e84db3493b64345b244146200df76e9/)
- [`file-write` (Python) with compression=uncompressed, dataset=fanniemae_2016Q4, file_type=feather, input_type=table](https://conbench.arrow-dev.org/compare/benchmarks/06a05e07287f7d7a80006ae8984f16a7...06a062d993e37b848000b7e6aa9a5600)
The [full Conbench report](https://github.com/apache/arrow/runs/76098268831) has more details. It also includes information about 67 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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
Re: [PR] GH-40742: [R] fix max_rows_per_group must be a positive number [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #49709:
URL: https://github.com/apache/arrow/pull/49709#discussion_r3189193499
##########
r/R/dataset-write.R:
##########
@@ -218,7 +218,12 @@ write_dataset <- function(
existing_data_behavior_opts <- c("delete_matching", "overwrite", "error")
existing_data_behavior <- match(match.arg(existing_data_behavior), existing_data_behavior_opts) - 1L
- if (!missing(max_rows_per_file) && missing(max_rows_per_group) && max_rows_per_group > max_rows_per_file) {
+ if (
+ !missing(max_rows_per_file) &&
+ missing(max_rows_per_group) &&
+ max_rows_per_file > 0 &&
Review Comment:
Yup!
--
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
Re: [PR] GH-40742: [R] fix max_rows_per_group must be a positive number [arrow]
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on code in PR #49709:
URL: https://github.com/apache/arrow/pull/49709#discussion_r3189043147
##########
r/R/dataset-write.R:
##########
@@ -218,7 +218,12 @@ write_dataset <- function(
existing_data_behavior_opts <- c("delete_matching", "overwrite", "error")
existing_data_behavior <- match(match.arg(existing_data_behavior), existing_data_behavior_opts) - 1L
- if (!missing(max_rows_per_file) && missing(max_rows_per_group) && max_rows_per_group > max_rows_per_file) {
+ if (
+ !missing(max_rows_per_file) &&
+ missing(max_rows_per_group) &&
+ max_rows_per_file > 0 &&
Review Comment:
This line `max_rows_per_file > 0` is the one condition that was change, yeah? The rest is whitespace?
--
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