Skip to content

GH-46516: [CI][Python] Force Cython>3.1.1 for docs builds - #46770

Merged
kou merged 5 commits into
apache:mainfrom
raulcd:GH-46516
Jun 10, 2025
Merged

kou merged 5 commits into
apache:mainfrom
raulcd:GH-46516

Conversation

@raulcd

@raulcd raulcd commented Jun 10, 2025 •

Copy link
Copy Markdown
Member

Rationale for this change

Due to a change of behaviour on Cython for 3.1 the docstrings generated fail when being tested with numpydoc. The issue was fixed on Cython==3.1.2.

What changes are included in this PR?

  • Pin Cython>3.1.1 for the docs requirements (both conda and pip)
  • Fix doctest failure that was introduced in the interim when the CI job had been failing.

Are these changes tested?

Via CI.

Are there any user-facing changes?

No

@github-actions

Copy link
Copy Markdown

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

Comment thread ci/scripts/python_build.sh Resolved
Comment thread ci/scripts/python_build.sh Outdated
# If we are building docs we force Cython>3.1.1 to avoid numpydoc
# failures due to Cython docstring being wrongly generated.
if [ "${BUILD_DOCS_PYTHON}" == "ON" ]; then
${PYTHON:-python} -m pip install --upgrade Cython>3.1.1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will that work properly if Cython was installed using conda?
Also, is this necessary at all?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will that work properly if Cython was installed using conda?

It does fix the existing failures which install Cython through conda, so the answer seems to be yes (without this it still fails) because the cached docker image installed Cython 3.1.1

Also, is this necessary at all?

I don't think we should merge that specific change, I was validating the existing failing CI jobs.
I think the bests course of action would be to force rebuild the underlying docker image so it is not caches and it installs Cython 3.1.2. I am asking on Zulip if someone has access to docker.io/apache/arrow-dev as rebuilding the image would fix the issue

@raulcd raulcd changed the title GH-46516: [CI][Python] Force Cython>3.1.1 if BUILD_DOCS_PYTHON=ON GH-46516: [CI][Python] Force Cython>3.1.1 for docs builds Jun 10, 2025
@raulcd
raulcd marked this pull request as ready for review June 10, 2025 12:30
@raulcd
raulcd requested a review from pitrou June 10, 2025 12:30

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just one suggestion

Comment thread ci/conda_env_sphinx.txt Resolved
Comment thread ci/conda_env_sphinx.txt Resolved

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@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 d2a8b78.

There were 70 benchmark results with an error:

There were no benchmark performance regressions. 🎉

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

alinaliBQ pushed a commit to Bit-Quill/arrow that referenced this pull request Jun 17, 2025
…he#46770)

### Rationale for this change

Due to a change of behaviour on Cython for 3.1 the docstrings generated fail when being tested with numpydoc. The issue was fixed on Cython==3.1.2.
- cython/cython#6904

### What changes are included in this PR?

- Pin Cython>3.1.1 for the docs requirements (both conda and pip)
- Fix doctest failure that was introduced in the interim when the CI job had been failing.

### Are these changes tested?

Via CI.

### Are there any user-facing changes?

No

* GitHub Issue: apache#46516

Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
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.

3 participants