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

GH-46516: [CI][Python] Force Cython>3.1.1 for docs builds#46770
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.

@github-actions github-actions Bot added the awaiting committer review Awaiting committer review label Jun 10, 2025
Comment thread ci/scripts/python_build.sh Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jun 10, 2025
@github-actions github-actions Bot added Component: Python awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 10, 2025
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

@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Jun 10, 2025
@github-actions github-actions Bot added Component: Documentation awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 10, 2025
@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
Comment thread ci/conda_env_sphinx.txt
@github-actions github-actions Bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Jun 10, 2025

@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

@github-actions github-actions Bot removed the awaiting changes Awaiting changes label Jun 10, 2025
@kou kou merged commit d2a8b78 into apache:main Jun 10, 2025
27 of 28 checks passed
@github-actions github-actions Bot added the awaiting merge Awaiting merge label Jun 10, 2025
@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 <raulcumplido@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
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