-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-49150: [Doc][CI][Python] Doctests failing on rst files due to pandas 3+ #49088
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.
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
Conversation
060eab3 to
f14ade1
Compare
docs/source/python/data.rst
Outdated
| 7 baz | ||
| dtype: category | ||
| Categories (3, object): ['foo', 'bar', 'baz'] | ||
| Categories (3, ...): ['foo', 'bar', 'baz'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I think if we know the doctests will run on an instance with pandas>=3, I would just update those to use pandas 3 output, because that will be less confusing for people reading the docs ("what do those 3 dots mean here?").
| Categories (3, ...): ['foo', 'bar', 'baz'] | |
| Categories (3, str): ['foo', 'bar', 'baz'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to start testing on Pandas 3 exclusively? I believe we have both testing now. (One is not triggered for this PR because the scope is smaller)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do agree to stop doctest on Pandas 2.x.y then we should also update CI appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For general tests (unit tests) we certainly have to test with various pandas versions (i.e. whichever range we claim to support), but personally I think for doctests it is perfectly fine to just have those passing in one specific environment (they are not meant as actual proper tests, but only to ensure the docs are correct and kept up to date)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this approach. I've reverted changes so that this now tests expected pandas 3 behavior.
Additionally this also updates Python to 3.11 which then makes pandas-latest==3+.
4d0a733 to
c84c874
Compare
c84c874 to
88da613
Compare
88da613 to
6482c71
Compare
dcbca14 to
130bfa4
Compare
|
Thanks for the review @jorisvandenbossche ! Could you take another look before the merge? |
98c6735 to
f22181a
Compare
f22181a to
a951f71
Compare
|
|
|
"Docs / AMD64 Debian 12 Complete Documentation" seems to be failing for CRAN reasons: -- Building articles -----------------------------------------------------------
Warning message:
@examplesIf condition "arrow_with_dataset() &&
identical(Sys.getenv(\"NOT_CRAN\"), \"true\")" is "FALSE"
---
Backtrace:
1. pkgdown::build_site(install = FALSE)
2. pkgdown:::build_site_external(pkg = pkg, examples = examples, run_dont_run = run_d...
3. callr::r(function(..., cli_colors, hyperlinks) { ...
4. callr:::get_result(output = out, options)
5. callr:::throw(callr_remote_error(remerr, output), parent = fix_msg(remerr[[3]]))
---
Subprocess backtrace:
1. pkgdown::build_site(...)
2. pkgdown:::build_site_local(pkg = pkg, examples = examples, run_dont_run = run_dont...
3. pkgdown::build_articles(pkg, lazy = lazy, override = override, quiet = quiet, ...
4. pkgdown::build_articles_index(pkg)
5. pkgdown::render_page(pkg, "article-index", data = data_articles_index(pkg), ...
6. pkgdown:::render_page_html(pkg, name = name, data = data, depth = depth)
7. pkgdown:::modify_list(data_template(pkg, depth = depth), data)
8. pkgdown:::data_articles_index(pkg)
9. pkgdown:::config_abort(pkg, "{length(missing)} vignette{?s} missing from index: {....
10. cli::cli_abort(message, ..., call = call, .envir = .envir)
11. | rlang::abort(message, ..., call = call, use_cli_format = TRUE, ...
12. | rlang:::signal_abort(cnd, .file)
13. | base::signalCondition(cnd)
14. global (function (e) ...
Execution halted
Error: `docker compose --file=/home/runner/work/arrow/arrow/compose.yaml run --rm debian-docs` exited with a non-zero exit code 1, see the process cc @thisisnic |
I just merged a fix, if you rebase it should be good for the R docs now :) |
a951f71 to
ae226b4
Compare
|
Thanks @thisisnic, rebasing seems to fix the CI! Merging. |
Fixes: #49150
See #48619 (comment)
Rationale for this change
Fix CI failures
What changes are included in this PR?
Tests are made more general to allow for Pandas 2 and Pandas 3 style string types
Are these changes tested?
By CI
Are there any user-facing changes?
No