Skip to content

GH-39638: [Docs][R] Add r-universe instructions#44033

Merged
thisisnic merged 4 commits intoapache:mainfrom
grantmcdermott:patch-1
Sep 14, 2024
Merged

GH-39638: [Docs][R] Add r-universe instructions#44033
thisisnic merged 4 commits intoapache:mainfrom
grantmcdermott:patch-1

Conversation

@grantmcdermott
Copy link
Copy Markdown
Contributor

@grantmcdermott grantmcdermott commented Sep 9, 2024

Copy link
Copy Markdown
Contributor

@eitsupi eitsupi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The apache R-universe is managed here, but it seems unclear at this time whether it is for release builds or edge builds.
https://github.com/r-universe-org/apache.r-universe.dev

If it is for edge builds, R-multiverse may be more suitable for installing release builds.
Or it may be worth updating the apache universe to show the same version as the CRAN release.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 10, 2024
grantmcdermott and others added 2 commits September 9, 2024 20:40
Co-authored-by: eitsupi <50911393+eitsupi@users.noreply.github.com>
@grantmcdermott
Copy link
Copy Markdown
Contributor Author

The apache R-universe is managed here, but it seems unclear at this time whether it is for release builds or edge builds. https://github.com/r-universe-org/apache.r-universe.dev

If it is for edge builds, R-multiverse may be more suitable for installing release builds. Or it may be worth updating the apache universe to show the same version as the CRAN release.

Hmmm. Fair point. Let's wait for the arrow devs to weigh in. It may be easiest to keep the current managed repo and just remove the "development" language.

@thisisnic
Copy link
Copy Markdown
Member

Thanks for making this PR @grantmcdermott! Just to confirm, we do use it for release builds and not dev builds. This addition is still welcome though, just with that bit updated.

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Sep 10, 2024
Copy link
Copy Markdown
Member

@amoeba amoeba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some style/spelling changes. I agree with the sentiment to change the language about development versions to release here too but didn't include them now.

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Sep 10, 2024
@grantmcdermott
Copy link
Copy Markdown
Contributor Author

grantmcdermott commented Sep 10, 2024

Thanks for the feedback all. I've actioned the requested changes (modulo the uppercase suggestion). Let me know if you want changes to the wording.

Some other points/thoughts:

  • I didn't mention anything about R-universe potentially guaranteeing a richer feature set because of CRAN restrictions (e.g., like we've experienced on MacOS with certain releases in the past w.r.t. S3 access etc.) I feel that this is a potentially important point that users should know about. But you all have better visibility to this stuff on the dev side.

  • Looking over the README, I feel like there's quite a bit of unnecessary repetition, made worse by inconsistent referencing the the upstream C++ docs and the R package website. For example...

    To learn more about the Apache Arrow project, see the parent documentation of the Arrow Project. The Arrow project provides functionality for a wide range of data analysis tasks to store, process and move data fast. See the read/write article to learn about reading and writing data files, data wrangling to learn how to use dplyr syntax with arrow objects, and the function documentation for a full list of supported functions within dplyr queries.

    (note the mixed links to the upstream docs and the individual R vignettes)
    ... is followed by a dedicated "What can the arrow package do?" only a few paragraphs later. Better IMO to consolidate this information in one place and generally prioritize the R package documentation/website above the upstream docs. I'm happy to take a crack at consolidation as part of this PR, but I imagine that you would prefer a separate PR. LMK.

@eitsupi
Copy link
Copy Markdown
Contributor

eitsupi commented Sep 11, 2024

Maybe the PR title is needed updated to something like GH-39638: [Docs][R] ...?

@amoeba amoeba changed the title MINOR: [R] Add r-universe instructions MINOR: [Docs][R] Add r-universe instructions Sep 11, 2024
@amoeba amoeba changed the title MINOR: [Docs][R] Add r-universe instructions GH-39638: [Docs][R] Add r-universe instructions Sep 11, 2024
@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #39638 has been automatically assigned in GitHub to PR creator.

@amoeba
Copy link
Copy Markdown
Member

amoeba commented Sep 11, 2024

Good call @eitsupi, I've changed the title and updated the related issue too. These changes now look good to me @grantmcdermott, thanks. And to your point about potential further improvements, those are probably best as separate issue.

@thisisnic do you want one more look at this PR?

Copy link
Copy Markdown
Member

@thisisnic thisisnic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful, thanks so much!

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Sep 14, 2024
@conbench-apache-arrow
Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 3eb4135.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 21 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Docs][R] Mention installing from r-universe binaries in readme

4 participants