Skip to content

Conversation

@AlenkaF
Copy link
Member

@AlenkaF AlenkaF commented May 23, 2022

This PR adds a CI job to test python docstrings with doctest.

It can be tested with archery docker run conda-python-docs.

@github-actions
Copy link

@AlenkaF AlenkaF changed the title ARROW-16018: [Doc][Python] Run doctests on Python docstring examples ARROW-16018: [Doc][Python] Run doctests on Python docstring examples (CI job) May 23, 2022
@jorisvandenbossche
Copy link
Member

CI shows:

 ValueError: Found errors with docker-compose:
 - Service `conda-python-doctest` is defined in `services` but not in `x-hierarchy`

@jorisvandenbossche
Copy link
Member

I am wondering if we can include the doctests in the existing sphinx doc build as a separate step, to avoid adding yet another build in the regular CI (also, setting up the docker and building arrow/pyarrow takes longer as the actual tests). Although for local use, it might be nice you can run it separately.

cc @raulcd

@AlenkaF
Copy link
Member Author

AlenkaF commented May 24, 2022

Planning to correct the CI errors asap.

@AlenkaF
Copy link
Member Author

AlenkaF commented May 24, 2022

I get this error locally also, not sure if I have to add permission on the file?

#6 [3/3] RUN /arrow/ci/scripts/install_doctest_cython.sh ${doctest-cython}
#6 sha256:ee58eeb773027f937695e90f7996eddb8d5709efed5beae96508fe15beda15bd
#6 0.594 mesg: ttyname failed: Inappropriate ioctl for device
#6 1.628 /bin/bash: /arrow/ci/scripts/install_doctest_cython.sh: Permission denied
#6 ERROR: executor failed running [/bin/bash -c -l /arrow/ci/scripts/install_doctest_cython.sh ${doctest-cython}]: exit code: 126

@raulcd
Copy link
Member

raulcd commented May 24, 2022

I get this error locally also, not sure if I have to add permission on the file?

I've been able to fix it by changing the permissions from the file and make it executable:

$ cd ci/scripts/
$ chmod +x install_doctest_cython.sh

@raulcd
Copy link
Member

raulcd commented May 24, 2022

I am wondering if we can include the doctests in the existing sphinx doc build as a separate step, to avoid adding yet another build in the regular CI (also, setting up the docker and building arrow/pyarrow takes longer as the actual tests). Although for local use, it might be nice you can run it separately.

cc @raulcd

That is a pretty good point. I see benefits and drawbacks on both cases but probably at the point we are right now, were we struggle to get runners on GitHub and docker cache is not working correctly, it might make more sense to add an extra step to the existing Sphinx docs build instead of creating a new CI job as you are suggesting. We have a couple of ideas in mind to extend the capacity of our GitHub runners and improve the Docker caching but they will take some time to be done.

@amol-
Copy link
Member

amol- commented May 25, 2022

Seems to be failing some tests at the moment

==================================== ERRORS ====================================
_ ERROR collecting opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/cuda.py _
opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/cuda.py:21: in <module>
    from pyarrow._cuda import (Context, IpcMemHandle, CudaBuffer,
E   ModuleNotFoundError: No module named 'pyarrow._cuda'
_ ERROR collecting opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/substrait.py _
opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/substrait.py:18: in <module>
    from pyarrow._substrait import (  # noqa
E   ModuleNotFoundError: No module named 'pyarrow._substrait'
_ ERROR collecting opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/tests/deserialize_buffer.py _
opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/tests/deserialize_buffer.py:24: in <module>
    with open(sys.argv[1], 'rb') as f:
E   FileNotFoundError: [Errno 2] No such file or directory: '-r'
_ ERROR collecting opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/tests/read_record_batch.py _
opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/tests/read_record_batch.py:24: in <module>
    with open(sys.argv[1], 'rb') as f:
E   FileNotFoundError: [Errno 2] No such file or directory: '-r'
=============================== warnings summary ===============================

I don't think we should be using sys.argv in the examples, as that will be the arguments of pytest, not the arguments of the example.

@jorisvandenbossche
Copy link
Member

Yes, those failures are handled in #13199 already. The current plan is to merge that first (soon, once CI is green again), and then update this PR.

jorisvandenbossche pushed a commit that referenced this pull request May 25, 2022
…(--doctest-modules)

A series of 3 PRs add `doctest` functionality to ensure that docstring examples are actually correct (and keep being correct).

- [x] Add `--doctest-module`
- [x] Add `--doctest-cython` #13204
- [x] Create a CI job #13216

This PR can be tested with `pytest --doctest-modules python/pyarrow`.

Closes #13199 from AlenkaF/ARROW-16018

Lead-authored-by: Alenka Frim <frim.alenka@gmail.com>
Co-authored-by: Alenka Frim <AlenkaF@users.noreply.github.com>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@AlenkaF
Copy link
Member Author

AlenkaF commented May 26, 2022

This should be ready to merge.
@raulcd I think Travis failed build is not connected, but am not sure. Can I ask for your opinion on this?

@raulcd
Copy link
Member

raulcd commented May 26, 2022

This should be ready to merge. @raulcd I think Travis failed build is not connected, but am not sure. Can I ask for your opinion on this?

The failures on travis don't seem related to the change to me either.

@amol- amol- closed this in 7fda23c May 26, 2022
@ursabot
Copy link

ursabot commented May 26, 2022

Benchmark runs are scheduled for baseline = 841e905 and contender = 7fda23c. 7fda23c is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️1.24% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.99% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 7fda23c3 ec2-t3-xlarge-us-east-2
[Finished] 7fda23c3 test-mac-arm
[Finished] 7fda23c3 ursa-i9-9960x
[Finished] 7fda23c3 ursa-thinkcentre-m75q
[Finished] 841e905f ec2-t3-xlarge-us-east-2
[Failed] 841e905f test-mac-arm
[Finished] 841e905f ursa-i9-9960x
[Finished] 841e905f ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. 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

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.

5 participants