-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-36730: [Python] Add support for Cython 3.0.0 #37097
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
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or In the case of PARQUET issues on JIRA the title also supports: See also: |
88db07c to
14c4ce9
Compare
14c4ce9 to
62f3ab6
Compare
|
Potentially related cloudpickle issue? cloudpipe/cloudpickle#506 Edit: Nope. Cython 3.0.0 changed the default compiler directive "binding" from False to True. Setting back to false fixes the cloudpickle test. |
|
I've copied #36745 PR description. |
59e70f6 to
c8e6a19
Compare
|
@github-actions crossbow submit python |
|
Revision: c8e6a19063768c7e6a08a2e7b878b1bb4f28a9cc Submitted crossbow builds: ursacomputing/crossbow @ actions-ba5c455e80 |
|
|
|
Revision: 6f32fcf Submitted crossbow builds: ursacomputing/crossbow @ actions-165015566a |
|
I've separated out the enablement of Cython 3 into a separate diff: #37743 Now, we can merge Cython 3 support and then enable Cython 3 once Cython 3.0.3 is released. |
|
IMO this can be merged now! |
python/pyarrow/_dataset.pyx
Outdated
| """A Dataset created from a list of paths on a particular filesystem. | ||
| @staticmethod | ||
| def from_paths(paths, schema=None, format=None, filesystem=None, | ||
| partitions=None, root_partition=None): |
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.
Hmm... classmethod doesn't work correctly on Cython 3 anymore? Is it a known issue? Or is this change actually not needed?
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.
classmethod does work, I can change it back. I tried switching to staticmethod when numpydocs were failing, but it turns out it was the comment that was causing numpydoc parsing errors. I thought staticmethod was a slight improvement since cls wasn't actually used in the classmethod.
AlenkaF
left a comment
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.
Thank you for working on this Dane!! Great work 👍
|
@github-actions crossbow submit -g python |
|
|
@github-actions crossbow submit python |
|
|
We need to fix #37803 to use |
Thanks! Got it now =) |
|
@github-actions crossbow submit -g python |
|
Revision: 3c4f581 Submitted crossbow builds: ursacomputing/crossbow @ actions-d63bfc88b3 |
|
None of the failures look related, wil merge. Thanks again Dane! |
|
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit e83c23b. There was 1 benchmark result indicating a performance regression:
The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them. |
### Rationale for this change The Cython 3.0.0 upgrade apache#37097 is triggering numpydoc errors for these missing docstrings. ### What changes are included in this PR? * Docstrings added to Cython functions that omitted them ### Are these changes tested? Yes, locally. ### Are there any user-facing changes? User-facing documentation is added. * Closes: apache#37217 Lead-authored-by: Dane Pitkin <[email protected]> Co-authored-by: Dane Pitkin <[email protected]> Co-authored-by: Alenka Frim <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
### Rationale for this change Cython 3.0.0 is the latest release. PyArrow should work with Cython 3.0.0. **Cython 3 is not enabled in this diff.** ### What changes are included in this PR? * Don't use `vector[XXX]&&` * Add a declaration for `postincrement` * See also: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#c-postincrement-postdecrement-operator * Ignore `C4551` warning (function call missing argument list) with MSVC * See also: cython/cython#4445 * Add missing `const` to `CLocation`'s static methods. * Don't use `StopIteration` to stop generator * See also: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#python-3-syntax-semantics * non-extern `cdef` functions will now propagate python exceptions automatically unless explicitly labeled `noexcept` * Function binding in cython is now enabled by default. Class methods that are used as wrappers for pickling should be converted to staticmethods. * Numpydocs now validates more Cython 3 objects than Cython <3 * Enum types are now being validated, and some unhelpful validation checks on Enums are now ignored * Added a cython <3 nightly CI job Note: * Cython 3.0.0, 3.0.1, 3.0.2 has an issue when compiling with debug mode cython/cython#5552 ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: apache#36730 Lead-authored-by: Dane Pitkin <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Co-authored-by: Dane Pitkin <[email protected]> Signed-off-by: AlenkaF <[email protected]>
### Rationale for this change Cython 3.0.0 is the latest release. PyArrow should work with Cython 3.0.0. **Cython 3 is not enabled in this diff.** ### What changes are included in this PR? * Don't use `vector[XXX]&&` * Add a declaration for `postincrement` * See also: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#c-postincrement-postdecrement-operator * Ignore `C4551` warning (function call missing argument list) with MSVC * See also: cython/cython#4445 * Add missing `const` to `CLocation`'s static methods. * Don't use `StopIteration` to stop generator * See also: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html#python-3-syntax-semantics * non-extern `cdef` functions will now propagate python exceptions automatically unless explicitly labeled `noexcept` * Function binding in cython is now enabled by default. Class methods that are used as wrappers for pickling should be converted to staticmethods. * Numpydocs now validates more Cython 3 objects than Cython <3 * Enum types are now being validated, and some unhelpful validation checks on Enums are now ignored * Added a cython <3 nightly CI job Note: * Cython 3.0.0, 3.0.1, 3.0.2 has an issue when compiling with debug mode cython/cython#5552 ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: apache#36730 Lead-authored-by: Dane Pitkin <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Co-authored-by: Dane Pitkin <[email protected]> Signed-off-by: AlenkaF <[email protected]>
Rationale for this change
Cython 3.0.0 is the latest release. PyArrow should work with Cython 3.0.0. Cython 3 is not enabled in this diff.
What changes are included in this PR?
vector[XXX]&&postincrementC4551warning (function call missing argument list) with MSVCconsttoCLocation's static methods.StopIterationto stop generatorcdeffunctions will now propagate python exceptions automatically unless explicitly labelednoexceptNote:
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.