Skip to content

GH-40535: [Docs][R] Set RETICULATE_PYTHON_ENV in order to find pyarrow #40571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
kou merged 2 commits into from
Mar 16, 2024
Merged

GH-40535: [Docs][R] Set RETICULATE_PYTHON_ENV in order to find pyarrow #40571

kou merged 2 commits into from
Mar 16, 2024

Conversation

@raulcd
Copy link
Member

@raulcd raulcd commented Mar 15, 2024 •
edited by kou
Loading

Rationale for this change

With the changes in #40455 reticulate can't find pyarrow.

What changes are included in this PR?

Set RETICULATE_PYTHON_ENV to ARROW_PYTHON_VENV.
See https://rstudio.github.io/reticulate/articles/versions.html#order-of-discovery why we can use RETICULATE_PYTHON_ENV.

This also simplifies the current configurations:

  • Install Meson by apt-get.
  • Use ARROW_PYTHON_VENV to install Sphinx and related packages.

Are these changes tested?

Via archery.

Are there any user-facing changes?

No

@raulcd raulcd requested review from assignUser and kou as code owners March 15, 2024 11:17
@raulcd
Copy link
Member Author

raulcd commented Mar 15, 2024

@github-actions crossbow submit preview-docs

@github-actions
Copy link

github-actions bot commented Mar 15, 2024

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

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Mar 15, 2024
@github-actions
Copy link

github-actions bot commented Mar 15, 2024

Revision: 8352456

Submitted crossbow builds: ursacomputing/crossbow @ actions-50ffe92163

Task Status
preview-docs GitHub Actions

kou
kou reviewed Mar 15, 2024
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

(Sorry for not working on this today...)

ci/docker/linux-apt-docs.dockerfile Outdated
npm install -g yarn

# Expose ARROW_PYTHON_VENV from BASE
ENV ARROW_PYTHON_VENV /arrow-dev
Copy link
Member

@kou kou Mar 15, 2024

Choose a reason for hiding this comment

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

I think that we don't need this. (It should be inherited from the base image automatically.)

Copy link
Member

@kou kou Mar 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Or how about defining RETICULATE_PYTHON_ENV here?

ENV RETICULATE_PYTHON_ENV=${ARROW_PYTHON_VENV}

ci/scripts/r_build.sh Outdated

if [ "${BUILD_DOCS_R}" == "ON" ]; then
if [ -n "${ARROW_PYTHON_VENV:-}" ]; then
RETICULATE_PYTHON_ENV=${ARROW_PYTHON_VENV}
Copy link
Member

@kou kou Mar 15, 2024

Choose a reason for hiding this comment

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

Suggested change
RETICULATE_PYTHON_ENV=${ARROW_PYTHON_VENV}
export RETICULATE_PYTHON_ENV=${ARROW_PYTHON_VENV}

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Mar 15, 2024
@raulcd
Copy link
Member Author

raulcd commented Mar 15, 2024

Sorry, I thought this was working but is still not finding pyarrow:

Error: 
! in callr subprocess.
Caused by error in `map(.x, .f, ..., .progress = .progress)`:
! i In index: 1.
Caused by error in `render_rmarkdown()`:
! Failed to render RMarkdown
Caused by error:
! in callr subprocess.
Caused by error:
! ModuleNotFoundError: No module named 'pyarrow'
Run `reticulate::py_last_error()` for details.

but I can see on my local logs:

+ '[' -n /arrow-dev ']'
+ export RETICULATE_PYTHON_ENV=/arrow-dev
+ RETICULATE_PYTHON_ENV=/arrow-dev

@kou
Copy link
Member

kou commented Mar 15, 2024

Can we try #40571 (comment) ?
(Can I push a commit to this branch?)

@raulcd
Copy link
Member Author

raulcd commented Mar 15, 2024

Can we try #40571 (comment) ? (Can I push a commit to this branch?)

Yes, you can push to this branch! No problem about that

@kou
Copy link
Member

kou commented Mar 16, 2024

@github-actions crossbow submit preview-docs

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 16, 2024
@github-actions
Copy link

github-actions bot commented Mar 16, 2024

Revision: 655388d

Submitted crossbow builds: ursacomputing/crossbow @ actions-a255e1a39a

Task Status
preview-docs GitHub Actions

kou
kou approved these changes Mar 16, 2024
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou merged commit b448b33 into apache:main Mar 16, 2024
@kou kou removed the awaiting change review Awaiting change review label Mar 16, 2024
@kou kou mentioned this pull request Mar 16, 2024
Closed
@github-actions github-actions bot added the awaiting merge Awaiting merge label Mar 16, 2024
@conbench-apache-arrow
Copy link

conbench-apache-arrow bot commented Mar 17, 2024

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

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 31 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

Reviewers

@kou kou kou approved these changes

@assignUser assignUser Awaiting requested review from assignUser assignUser is a code owner

Assignees

No one assigned

Labels

awaiting merge Awaiting merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@raulcd @kou