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/12/09 19:11:51 UTC
[GitHub] [arrow] dragosmg opened a new pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
dragosmg opened a new pull request #11921:
URL: https://github.com/apache/arrow/pull/11921
@jonkeane @nealrichardson @thisisnic draft 1: am I missing dependencies?
Having them separately means we need to run 2 lines of code:
```
r> install_deps(dependencies = "Config/Needs/website")
r> install_deps(dependencies = "Config/Needs/dev")
```
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-990144306
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008797522
Revision: a3a759d0c4e5c6be968aad9c6452ca091c4491c2
Submitted crossbow builds: [ursacomputing/crossbow @ actions-1382](https://github.com/ursacomputing/crossbow/branches/all?query=actions-1382)
|Task|Status|
|----|------|
|test-ubuntu-default-docs|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-1382-azure-test-ubuntu-default-docs)|
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1057040753
Benchmark runs are scheduled for baseline = 4aaf1f2e7e741fae5a8b02a514dcc891083d57b7 and contender = d9a5c1697ea44445d38f9490116cc977f8785fdc. d9a5c1697ea44445d38f9490116cc977f8785fdc 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/0fa3d5cdb2984d0784c4461153f9e519...d407f6656c9a46cfaf17db4e97255a2e/)
[Finished :arrow_down:0.25% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/6226535b2b454d31a3a07f594f1d386c...0bd849146fbc4e4f8e5d6185e6e4b118/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/25c6ba066a4142548735f0cfb18db54a...ba514d6657b94f31954a5ea946d947a5/)
[Finished :arrow_down:0.94% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5104a3cc178648f79adbb16daae87d16...ccff6fce062747459599c2653c7d5406/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] jonkeane commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-991012164
This seems fine, would you mind also changing / adding to the developer docs to reflect what this change would mean? Same for the make file: could you add in what would need to change there?
You also should look through the repo and see if there are other places (i.e. in our CI) that could benefit from this (presumably, one of the benefits is that we can control what versions / where from those are installed from one place now rather than needing to do it in a bunch of different places?)
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008798846
I think I am on the right track as the documentation job is failing with a code 3 which indicates we are on the correct branch of the conditional statement in `r_deps.sh`
```
elif [ ${R_DEV_DEPS} = "website" ]; then
exit 3
${R_BIN} -e "remotes::install_deps(dependencies = 'Config/Needs/website')"
```
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1006612212
@thisisnic & @jonkeane would you mind having a look? 🙏🏻
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-990216592
Bumped into this by accident, after installing the dev version of {roxygen2} with the above command.
<img width="964" alt="image" src="https://user-images.githubusercontent.com/13176361/145469838-d3bc0adb-d6c6-4e5f-b5cf-f6d16d1363f3.png">
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1035560398
Given the results of the [little investigation](https://issues.apache.org/jira/browse/ARROW-15299) into the `dependencies` argument for `remotes::install_deps()` (reproduced below). What direction should we take this PR in?
Options:
* do nothing - leave things as they are (i.e. not use the `Config/Needs/...` fields in DESCRIPTION -> close this PR without merging any of the proposed changes
* do something - cherry pick the PR and mostly keep the parts that improve a bit the documentation
* still make use of the `Config/Needs/...` fields as it allows us to keep some "workflow" dependencies closer to the code - keep the PR as is (maybe supplement calls with `dependencies = c("hard", "Config/Needs/website")` with `dependencies = TRUE`.
* do something else.
What do you think? @jonkeane @nealrichardson @thisisnic @paleolimbot @ianmcook and others 😄 .
=====================
Investigation results:
What we would need is a combination between `dependencies = TRUE` and `dependencies = "Config/Needs/website"`. I ran the experiment and the results are below:
test | description | number of packages installed
:--: | -- | :--:
1 | dependencies = TRUE | 64
2 | dependencies = 'hard' | 12
3 | dependencies = c('hard', 'Config/Needs/website') | 61
4 | dependencies = 'soft' | 1819
5 | dependencies = c('soft', 'Config/Needs/website') | 1819
6 | dependencies = c('Config/Needs/website') | 1
I excluded `dependencies = "soft"` & combinations of `"soft"` since they are _too_ recursive and end up installing an excessive number of packages.
`dependencies = "hard"` & `dependencies = "Config/Needs/website"` are too narrow.
I compared `dependencies = TRUE` to `dependencies = c("hard", "Config/Needs/website")`. While the overall number of installed packages looks promising (64 vs 61), there isn't complete overlap.
indicator | value
-- |:--:
packages exclusive to test 1 | 21
packages exclusive to test 3 | 18
packages in common | 43
We can safely conclude that `dependencies = c("hard", "Config/Needs/website")` is **not equal** to `dependencies = TRUE` + `dependencies = c("Config/Needs/website")`.
We definitely need `"Config/Needs/website"` - in our case pkgdown and its dependencies, but in combination with `dependencies = "hard"` it is too narrow in scope, while with `dependencies = "soft"` it is much too wide.
How I got there?
* I removed all installed packages from the system library (with the exception of `"base"` and `"recommended"`packages)
* I installed remotes and rstudioapi in the system library
* each test installed packages to its own folder (configured via the R_LIBS_USER environment variable) with the R session being restarted for each run
* I then compared the contents of the 6 libraries
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008741422
@github-actions crossbow submit test-ubuntu-default-docs
--
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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008886621
I think the docs might be failing due to `remotes::install_deps(dependencies = c("Config/Needs/website"))` only installing `pkgdown` and not it's dependencies, one of which is `fs`.
--
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] thisisnic commented on a change in pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
thisisnic commented on a change in pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#discussion_r781995880
##########
File path: r/vignettes/developers/workflow.Rmd
##########
@@ -4,6 +4,22 @@
knitr::opts_chunk$set(error = TRUE, eval = FALSE)
```
+The Arrow R package uses several additional development tools:
+
+* `lintr` for code analysis
+ - for the time being, the R package uses a custom version of lintr - `jonkeane/lintr@arrow-branch`
+* `styler` for code styling
+* `pkgdown` for building the website
+* `roxygen2` for documenting the package
Review comment:
I really like how you've mentioned the packages up-front here; how about also adding links to their pkgdown sites when you click on their names?
##########
File path: r/vignettes/developers/workflow.Rmd
##########
@@ -34,7 +44,7 @@ pkgdown::build_site(preview=TRUE)
The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
-To run the [lintr](https://github.com/jimhester/lintr) locally, install the lintr package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file `ci/docker/linux-apt-lint.dockerfile` for the current status) and then run
+To run the [lintr](https://github.com/r-lib/lintr) locally, install the lintr package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file `ci/docker/linux-apt-lint.dockerfile` for the current status) and then run
Review comment:
```suggestion
To run the [lintr](https://github.com/r-lib/lintr) locally, install the lintr package (note, we currently use a fork that includes fixes not yet accepted upstream; see how lintr is being installed in the file `ci/docker/linux-apt-lint.dockerfile` for the current status) and then run
```
--
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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008886621
I think the docs are failing due to `remotes::install_deps(dependencies = c("Config/Needs/website"))` only installing `pkgdown` and not it's dependencies, one of which is `fs`.
--
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] eitsupi commented on a change in pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
eitsupi commented on a change in pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#discussion_r817667036
##########
File path: r/vignettes/developers/workflow.Rmd
##########
@@ -4,6 +4,23 @@
knitr::opts_chunk$set(error = TRUE, eval = FALSE)
```
+The Arrow R package uses several additional development tools:
+
+* [`lintr`](https://github.com/r-lib/lintr) for code analysis
+ - for the time being, the R package uses a custom version of lintr - `jonkeane/lintr@arrow-branch`
+* [`styler`](https://styler.r-lib.org) for code styling
+* [`pkgdown`](https://pkgdown.r-lib.org) for building the website
+* [`roxygen2`](https://roxygen2.r-lib.org) for documenting the package
+ - the R documentation uses the [`@examplesIf`](https://roxygen2.r-lib.org/articles/rd.html#functions) tag introduced in `roxygen2` version 7.1.2
+
+You can install all these additional dependencies by running:
+
+```{r, eval=FALSE}
Review comment:
If it is just a code block, the eval option is not needed.
```suggestion
```r
```
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1056884416
Thanks. I'd be keen to merge this as-is if that's ok.
--
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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-990216592
Bumped into this by accident, after installing the dev version of {roxygen2} with `install_deps()`.
<img width="964" alt="image" src="https://user-images.githubusercontent.com/13176361/145469838-d3bc0adb-d6c6-4e5f-b5cf-f6d16d1363f3.png">
--
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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1047863399
I think this PR boils down to whether we want dependencies hardcoded or not.
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1057040753
Benchmark runs are scheduled for baseline = 4aaf1f2e7e741fae5a8b02a514dcc891083d57b7 and contender = d9a5c1697ea44445d38f9490116cc977f8785fdc. d9a5c1697ea44445d38f9490116cc977f8785fdc 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/0fa3d5cdb2984d0784c4461153f9e519...d407f6656c9a46cfaf17db4e97255a2e/)
[Finished :arrow_down:0.25% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/6226535b2b454d31a3a07f594f1d386c...0bd849146fbc4e4f8e5d6185e6e4b118/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/25c6ba066a4142548735f0cfb18db54a...ba514d6657b94f31954a5ea946d947a5/)
[Finished :arrow_down:0.94% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5104a3cc178648f79adbb16daae87d16...ccff6fce062747459599c2653c7d5406/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-991022139
Not sure what to recommend in the developer docs. I ran
```
install_deps(".", dependencies = c("soft", "Config/Needs/1", "Config/Needs/2"))
```
and 40+ minutes later my R session was still busy installing (mostly binary) packages. `dependencies = "soft"` is the equivalent of `dependencies = TRUE` but something is not right with this picture.
I'd be tempted to suggest using pak as that uses a separate process and is much faster.
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1057040753
Benchmark runs are scheduled for baseline = 4aaf1f2e7e741fae5a8b02a514dcc891083d57b7 and contender = d9a5c1697ea44445d38f9490116cc977f8785fdc. d9a5c1697ea44445d38f9490116cc977f8785fdc 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/0fa3d5cdb2984d0784c4461153f9e519...d407f6656c9a46cfaf17db4e97255a2e/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/6226535b2b454d31a3a07f594f1d386c...0bd849146fbc4e4f8e5d6185e6e4b118/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/25c6ba066a4142548735f0cfb18db54a...ba514d6657b94f31954a5ea946d947a5/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5104a3cc178648f79adbb16daae87d16...ccff6fce062747459599c2653c7d5406/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] ursabot edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1057040753
Benchmark runs are scheduled for baseline = 4aaf1f2e7e741fae5a8b02a514dcc891083d57b7 and contender = d9a5c1697ea44445d38f9490116cc977f8785fdc. d9a5c1697ea44445d38f9490116cc977f8785fdc 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/0fa3d5cdb2984d0784c4461153f9e519...d407f6656c9a46cfaf17db4e97255a2e/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/6226535b2b454d31a3a07f594f1d386c...0bd849146fbc4e4f8e5d6185e6e4b118/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/25c6ba066a4142548735f0cfb18db54a...ba514d6657b94f31954a5ea946d947a5/)
[Finished :arrow_down:0.94% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5104a3cc178648f79adbb16daae87d16...ccff6fce062747459599c2653c7d5406/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008886621
I think the docs are failing due to `remotes::install_deps(dependencies = c("Config/Needs/website"))` only installing `pkgdown` and not its dependencies, one of which is `fs`.
The previous iteration was using `utils::install.packages()` which installs dependencies.
--
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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1006722558
There is a [new(ish) GHA workflow for installing dependencies](https://github.com/r-lib/actions/tree/v2-branch/setup-r-dependencies). Do we want to make use of it or should I aim to close this issue and then open a follow-up Jira for later? It looks like it allows us to control how `Config/Needs` are installed, via:
```
needs: |
website
coverage
```
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1006722558
There is a [new GHA workflow for installing dependencies](https://github.com/r-lib/actions/tree/v2-branch/setup-r-dependencies). Do we want to make use of it or should I aim to close this issue and then open a follow-up Jira for later? It looks like it allows us to control how `Config/Needs` are installed, via:
```
needs: |
website
coverage
```
--
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] thisisnic commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
thisisnic commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1007516733
> There is a [new(ish) GHA workflow for installing dependencies](https://github.com/r-lib/actions/tree/v2-branch/setup-r-dependencies). Do we want to make use of it or should I aim to close this issue and then open a follow-up Jira for later? It looks like it allows us to control how `Config/Needs` are installed, via:
>
> ```
> needs: |
> website
> coverage
> ```
I'd be inclined to look at it in a follow-up ticket, to keep things a bit simpler on the ticket and PR discussion
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008790700
@github-actions crossbow submit test-ubuntu-default-docs
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008886621
I think the docs might be failing due to `remotes::install_deps(dependencies = c("Config/Needs/webiste"))` only installing `pkgdown` and not it's dependencies, one of which is `fs`.
--
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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008886621
I think the docs are failing due to `remotes::install_deps(dependencies = c("Config/Needs/website"))` only installing `pkgdown` and not its dependencies, one of which is `fs`.
The previous iteration was using `base::install.packages()` which installs dependencies.
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1057040753
Benchmark runs are scheduled for baseline = 4aaf1f2e7e741fae5a8b02a514dcc891083d57b7 and contender = d9a5c1697ea44445d38f9490116cc977f8785fdc. d9a5c1697ea44445d38f9490116cc977f8785fdc 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/0fa3d5cdb2984d0784c4461153f9e519...d407f6656c9a46cfaf17db4e97255a2e/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/6226535b2b454d31a3a07f594f1d386c...0bd849146fbc4e4f8e5d6185e6e4b118/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/25c6ba066a4142548735f0cfb18db54a...ba514d6657b94f31954a5ea946d947a5/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5104a3cc178648f79adbb16daae87d16...ccff6fce062747459599c2653c7d5406/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. 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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008798846
I think I am on the right track as the docs job is failing with a code 3 which indicates we are on the correct branch of the conditional statement in `r_deps.sh`
```
elif [ ${R_DEV_DEPS} = "website" ]; then
exit 3
${R_BIN} -e "remotes::install_deps(dependencies = 'Config/Needs/website')"
```
--
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] dragosmg commented on a change in pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on a change in pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#discussion_r782045709
##########
File path: r/vignettes/developers/workflow.Rmd
##########
@@ -4,6 +4,22 @@
knitr::opts_chunk$set(error = TRUE, eval = FALSE)
```
+The Arrow R package uses several additional development tools:
+
+* `lintr` for code analysis
+ - for the time being, the R package uses a custom version of lintr - `jonkeane/lintr@arrow-branch`
+* `styler` for code styling
+* `pkgdown` for building the website
+* `roxygen2` for documenting the package
Review comment:
That sounds like a great idea. We might need to that in a separate PR as I don't think we are going to merge this PR anytime soon.
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1047863399
I think this PR can be boiled down to whether we want dependencies hardcoded or not.
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1048649736
Discussed yesterday with @jonkeane to keep only the documentation improvement bits of this PR.
--
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] dragosmg commented on a change in pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on a change in pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#discussion_r817673096
##########
File path: r/vignettes/developers/workflow.Rmd
##########
@@ -4,6 +4,23 @@
knitr::opts_chunk$set(error = TRUE, eval = FALSE)
```
+The Arrow R package uses several additional development tools:
+
+* [`lintr`](https://github.com/r-lib/lintr) for code analysis
+ - for the time being, the R package uses a custom version of lintr - `jonkeane/lintr@arrow-branch`
+* [`styler`](https://styler.r-lib.org) for code styling
+* [`pkgdown`](https://pkgdown.r-lib.org) for building the website
+* [`roxygen2`](https://roxygen2.r-lib.org) for documenting the package
+ - the R documentation uses the [`@examplesIf`](https://roxygen2.r-lib.org/articles/rd.html#functions) tag introduced in `roxygen2` version 7.1.2
+
+You can install all these additional dependencies by running:
+
+```{r, eval=FALSE}
Review comment:
Good catch. Thanks
--
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] thisisnic closed pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
thisisnic closed pull request #11921:
URL: https://github.com/apache/arrow/pull/11921
--
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] jonkeane commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-991080111
> 40+ minutes later my R session was still busy installing (mostly binary) packages
Indeed, something sounds not quite right about that or not the behavior we would ultimately want here.
As for what to recommend in the developer docs I like to approach it from the questions of: Who is our audience there? What do they need to accomplish with these docs? What skills are they coming in with? (there almost certainly are multiple answers here, so we'll likely need to thread that needle between the different groups) Sometimes I find that writing out what the docs are (or what are the commands that I would run to do this task and what do those look like? Here those _are_ the docs cause we're not writing a new function here, but the principle is the same) helps figure out if the UX feels right or if something is missing there
--
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] thisisnic commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
thisisnic commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1007516733
> There is a [new(ish) GHA workflow for installing dependencies](https://github.com/r-lib/actions/tree/v2-branch/setup-r-dependencies). Do we want to make use of it or should I aim to close this issue and then open a follow-up Jira for later? It looks like it allows us to control how `Config/Needs` are installed, via:
>
> ```
> needs: |
> website
> coverage
> ```
I'd be inclined to look at it in a follow-up ticket, to keep things a bit simpler on the ticket and PR discussion
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008855952
Revision: bb08a408246f972dedce8c42597d7c50284c08fa
Submitted crossbow builds: [ursacomputing/crossbow @ actions-1383](https://github.com/ursacomputing/crossbow/branches/all?query=actions-1383)
|Task|Status|
|----|------|
|test-ubuntu-default-docs|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-1383-azure-test-ubuntu-default-docs)|
--
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] dragosmg commented on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008854900
@github-actions crossbow submit test-ubuntu-default-docs
--
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] dragosmg edited a comment on pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg edited a comment on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008886621
I think the docs are failing due to `remotes::install_deps(dependencies = c("Config/Needs/website"))` only installing `pkgdown` and not its dependencies, one of which is `fs`.
--
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 #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#issuecomment-1008742252
Revision: c72ceea5cb9d4e2d3abd804f65437d8c534ac067
Submitted crossbow builds: [ursacomputing/crossbow @ actions-1381](https://github.com/ursacomputing/crossbow/branches/all?query=actions-1381)
|Task|Status|
|----|------|
|test-ubuntu-default-docs|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-1381-azure-test-ubuntu-default-docs)|
--
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] dragosmg commented on a change in pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on a change in pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#discussion_r783983716
##########
File path: r/vignettes/developers/workflow.Rmd
##########
@@ -34,7 +44,7 @@ pkgdown::build_site(preview=TRUE)
The R code in the package follows [the tidyverse style](https://style.tidyverse.org/). On PR submission (and on pushes) our CI will run linting and will flag possible errors on the pull request with annotations.
-To run the [lintr](https://github.com/jimhester/lintr) locally, install the lintr package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file `ci/docker/linux-apt-lint.dockerfile` for the current status) and then run
+To run the [lintr](https://github.com/r-lib/lintr) locally, install the lintr package (note, we currently use a fork that includes fixes not yet accepted upstream, see how lintr is being installed in the file `ci/docker/linux-apt-lint.dockerfile` for the current status) and then run
Review comment:
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] dragosmg commented on a change in pull request #11921: ARROW-12743 [R] Add DESCRIPTION fields for dev dependencies
Posted by GitBox <gi...@apache.org>.
dragosmg commented on a change in pull request #11921:
URL: https://github.com/apache/arrow/pull/11921#discussion_r783982988
##########
File path: r/vignettes/developers/workflow.Rmd
##########
@@ -4,6 +4,22 @@
knitr::opts_chunk$set(error = TRUE, eval = FALSE)
```
+The Arrow R package uses several additional development tools:
+
+* `lintr` for code analysis
+ - for the time being, the R package uses a custom version of lintr - `jonkeane/lintr@arrow-branch`
+* `styler` for code styling
+* `pkgdown` for building the website
+* `roxygen2` for documenting the package
Review comment:
Done, with the exception of {lintr} which does not have a pkgdown site - I linked to the GitHub.
--
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