You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Jefffrey (via GitHub)" <gi...@apache.org> on 2024/03/29 00:57:34 UTC
[PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Jefffrey opened a new pull request, #5566:
URL: https://github.com/apache/arrow-rs/pull/5566
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on code in PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#discussion_r1547901346
##########
arrow/src/pyarrow.rs:
##########
@@ -101,15 +106,19 @@ impl<T: ToPyArrow> IntoPyArrow for T {
}
}
-fn validate_class(expected: &str, value: &PyAny) -> PyResult<()> {
- let pyarrow = PyModule::import(value.py(), "pyarrow")?;
+fn validate_class(expected: &str, value: &Bound<PyAny>) -> PyResult<()> {
+ let pyarrow = PyModule::import_bound(value.py(), "pyarrow")?;
let class = pyarrow.getattr(expected)?;
- if !value.is_instance(class)? {
- let expected_module = class.getattr("__module__")?.extract::<&str>()?;
- let expected_name = class.getattr("__name__")?.extract::<&str>()?;
+ if !value.is_instance(&class)? {
+ let expected_module = class.getattr("__module__")?;
+ let expected_module = expected_module.extract::<&str>()?;
+ let expected_name = class.getattr("__name__")?;
+ let expected_name = expected_name.extract::<&str>()?;
let found_class = value.get_type();
- let found_module = found_class.getattr("__module__")?.extract::<&str>()?;
- let found_name = found_class.getattr("__name__")?.extract::<&str>()?;
+ let found_module = found_class.getattr("__module__")?;
+ let found_module = found_module.extract::<&str>()?;
+ let found_name = found_class.getattr("__name__")?;
+ let found_name = found_name.extract::<&str>()?;
Review Comment:
This also breaks for me when `gil-refs` is not enabled. From [this issue](https://github.com/PyO3/pyo3/pull/3928) it appears that `&str` lost `FromPyObject`:
> Finally, without the gil-refs feature we remove the existing implementation of FromPyObject for &str and instead implement this new trait. This works except for abi3 on old Python versions, where we have to settle for PyFunctionArgument only and allow users of .extract() to break.
>
> This does implement some very targeted breakage for specific considerations, but I wonder if this is necessary as the correct way to users off the GIL Ref API in a case where otherwise it's near impossible for them to realise this uses it.
The reason it is not caught by CI is because CI does not have any `abi...` feature enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2030832886
I'll try and update pylance to pyo3 0.21 using this branch tomorrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "abhiaagarwal (via GitHub)" <gi...@apache.org>.
abhiaagarwal commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2030493275
This PR doesn't enable `gil-refs`, so I'm not sure why the compile error wasn't caught in CI. Maybe there isn't any coverage for the pyarrow integration in the main arrow-rs package?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2033036661
> We can wait until a future PR but I think this will need to get fixed upstream at some point. I'm not sure there is any workaround we do other than use `gil-refs`. Changing the pyo3 dependency (in the integration test) like this:
>
> ```
> pyo3 = { version = "0.21", features = ["extension-module", "abi3-py38"] }
> ```
>
> will allow the issue to be reproduced.
Thanks for checking this :+1:
I'll add the `gil-refs` feature as part of this PR for now to mitigate breaking downstream consumers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "abhiaagarwal (via GitHub)" <gi...@apache.org>.
abhiaagarwal commented on code in PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#discussion_r1546816258
##########
arrow/src/pyarrow.rs:
##########
@@ -101,15 +106,19 @@ impl<T: ToPyArrow> IntoPyArrow for T {
}
}
-fn validate_class(expected: &str, value: &PyAny) -> PyResult<()> {
- let pyarrow = PyModule::import(value.py(), "pyarrow")?;
+fn validate_class(expected: &str, value: &Bound<PyAny>) -> PyResult<()> {
+ let pyarrow = PyModule::import_bound(value.py(), "pyarrow")?;
let class = pyarrow.getattr(expected)?;
- if !value.is_instance(class)? {
- let expected_module = class.getattr("__module__")?.extract::<&str>()?;
- let expected_name = class.getattr("__name__")?.extract::<&str>()?;
+ if !value.is_instance(&class)? {
+ let expected_module = class.getattr("__module__")?;
+ let expected_module = expected_module.extract::<&str>()?;
+ let expected_name = class.getattr("__name__")?;
+ let expected_name = expected_name.extract::<&str>()?;
let found_class = value.get_type();
- let found_module = found_class.getattr("__module__")?.extract::<&str>()?;
- let found_name = found_class.getattr("__name__")?.extract::<&str>()?;
+ let found_module = found_class.getattr("__module__")?;
+ let found_module = found_module.extract::<&str>()?;
+ let found_name = found_class.getattr("__name__")?;
+ let found_name = found_name.extract::<&str>()?;
Review Comment:
@Jefffrey these `extract`s should be changed to `extract_bound`. `extract` only exists with `gil-refs` enabled. I have no clue why the `extract` version is passing CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2056597614
Can someone confirm that this isn't a breaking API change to the arrow APIs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "abhiaagarwal (via GitHub)" <gi...@apache.org>.
abhiaagarwal commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2029701551
Thanks for this PR :)
Fyi, I pulled in this branch to update delta-rs (see delta-io/delta-rs#2363). When `gil-refs` is enabled it compiles successfully, but when `gil-refs` is disabled I get a traits bounds error.
```
error[E0277]: the trait bound `&str: PyClass` is not satisfied
--> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:114:47
|
114 | let expected_module = expected_module.extract::<&str>()?;
| ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
|
= help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
Cow<'a, [u8]>
Cow<'a, str>
&'a [u8]
= note: required for `&str` to implement `pyo3::FromPyObject<'_>`
= note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
--> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
|
1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
| ------- required by a bound in this associated function
1648 | where
1649 | T: FromPyObjectBound<'a, 'py>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
error[E0277]: the trait bound `&str: PyClass` is not satisfied
--> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:116:43
|
116 | let expected_name = expected_name.extract::<&str>()?;
| ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
|
= help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
Cow<'a, [u8]>
Cow<'a, str>
&'a [u8]
= note: required for `&str` to implement `pyo3::FromPyObject<'_>`
= note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
--> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
|
1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
| ------- required by a bound in this associated function
1648 | where
1649 | T: FromPyObjectBound<'a, 'py>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
error[E0277]: the trait bound `&str: PyClass` is not satisfied
--> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:119:41
|
119 | let found_module = found_module.extract::<&str>()?;
| ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
|
= help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
Cow<'a, [u8]>
Cow<'a, str>
&'a [u8]
= note: required for `&str` to implement `pyo3::FromPyObject<'_>`
= note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
--> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
|
1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
| ------- required by a bound in this associated function
1648 | where
1649 | T: FromPyObjectBound<'a, 'py>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
error[E0277]: the trait bound `&str: PyClass` is not satisfied
--> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:121:37
|
121 | let found_name = found_name.extract::<&str>()?;
| ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
|
= help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
Cow<'a, [u8]>
Cow<'a, str>
&'a [u8]
= note: required for `&str` to implement `pyo3::FromPyObject<'_>`
= note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
--> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
|
1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
| ------- required by a bound in this associated function
1648 | where
1649 | T: FromPyObjectBound<'a, 'py>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
```
The `FromPyObject` trait gets disabled without `gil-refs`, leading to the error. ([source](https://github.com/PyO3/pyo3/blob/63ba371db021985e01818694cc0987ee213fd4b9/src/conversions/std/slice.rs#L21-L43)) It should be able to be fixed by changing all the `extract` to `extract_bound` per the migration guide.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "abhiaagarwal (via GitHub)" <gi...@apache.org>.
abhiaagarwal commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2039660871
will do, apologies, I didn't get a chance to run a new build :) I'll hopefully get to it soon. Though I'll wait on DataFusion to update its dependency as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2041224716
> will do, apologies, I didn't get a chance to run a new build :) I'll hopefully get to it soon. Though I'll wait on DataFusion to update its dependency as well.
Just a note is you might be waiting a while as DataFusion will need to wait for arrow-rs to release a new version before it can bump it's own, I think?
Feel free to chip in on discussion of release cadence for arrow-rs here #5368
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "abhiaagarwal (via GitHub)" <gi...@apache.org>.
abhiaagarwal commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2033116274
Thanks @Jefffrey! I'll try a build by the end of the day with the new commit here and see if we can build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "abhiaagarwal (via GitHub)" <gi...@apache.org>.
abhiaagarwal commented on code in PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#discussion_r1546818013
##########
arrow/src/pyarrow.rs:
##########
@@ -101,15 +106,19 @@ impl<T: ToPyArrow> IntoPyArrow for T {
}
}
-fn validate_class(expected: &str, value: &PyAny) -> PyResult<()> {
- let pyarrow = PyModule::import(value.py(), "pyarrow")?;
+fn validate_class(expected: &str, value: &Bound<PyAny>) -> PyResult<()> {
+ let pyarrow = PyModule::import_bound(value.py(), "pyarrow")?;
let class = pyarrow.getattr(expected)?;
- if !value.is_instance(class)? {
- let expected_module = class.getattr("__module__")?.extract::<&str>()?;
- let expected_name = class.getattr("__name__")?.extract::<&str>()?;
+ if !value.is_instance(&class)? {
+ let expected_module = class.getattr("__module__")?;
+ let expected_module = expected_module.extract::<&str>()?;
+ let expected_name = class.getattr("__name__")?;
+ let expected_name = expected_name.extract::<&str>()?;
let found_class = value.get_type();
- let found_module = found_class.getattr("__module__")?.extract::<&str>()?;
- let found_name = found_class.getattr("__name__")?.extract::<&str>()?;
+ let found_module = found_class.getattr("__module__")?;
+ let found_module = found_module.extract::<&str>()?;
+ let found_name = found_class.getattr("__name__")?;
+ let found_name = found_name.extract::<&str>()?;
Review Comment:
Okay, a bit of digging, and a public `extract` actually does still exist, but it's been redirected to `extract_bound`. https://github.com/PyO3/pyo3/blob/c1f11fb4bddc836f820e0db3db514b4742b8a3e7/src/types/any.rs#L805
I have no clue what's causing my build to freak out
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2026821285
Perhaps @wjones127 or @kylebarron might be able to advise on this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2027480960
I haven't tried to upgrade my projects to 0.21 yet, but yes
> So it seems they are moving away from `&PyAny` is my understanding.
that's my understanding as well. I'd say that the PyArrow API should _probably_ follow suit. Maybe while upgrading to 0.21 is the best opportunity?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2039293810
Feel free to raise a new issue if you still encounter problems with this @abhiaagarwal @westonpace :+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2058726712
> Can someone confirm that this isn't a breaking API change to the arrow APIs?
I think it may be classified as a breaking change in that it'll require consumers to upgrade their pyo3 version, but in terms of API itself we preserve the old API (and just deprecate it)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#discussion_r1544002230
##########
arrow/src/pyarrow.rs:
##########
@@ -102,8 +102,8 @@ impl<T: ToPyArrow> IntoPyArrow for T {
}
fn validate_class(expected: &str, value: &PyAny) -> PyResult<()> {
- let pyarrow = PyModule::import(value.py(), "pyarrow")?;
- let class = pyarrow.getattr(expected)?;
+ let pyarrow = PyModule::import_bound(value.py(), "pyarrow")?;
+ let class = pyarrow.getattr(expected)?.into_gil_ref(); // TODO
Review Comment:
I'm not familiar with pyo3, so I was following the migration guide: https://pyo3.rs/v0.21.0/migration.html
Of particular note is:
> PyO3 0.21 introduces a new `Bound<'py, T>` smart pointer which replaces the existing "GIL Refs" API to interact with Python objects. For example, in PyO3 0.20 the reference `&'py PyAny` would be used to interact with Python objects. In PyO3 0.21 the updated type is `Bound<'py, PyAny>`.
So it seems they are moving away from `&PyAny` is my understanding. This raises a question of if the PyArrow API's should follow suit?
e.g. here, introduce a new function that uses `Bound`?
https://github.com/apache/arrow-rs/blob/5f639061d4880716e458fe1bcce70495054a40f3/arrow/src/pyarrow.rs#L141
For now I've used `into_gil_ref()` to get around this deprecation process, but I don't think it's meant to be a permanent solution.
I'll do some more reading, maybe it's been discussed in pyo3 before somewhere :thinking:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2030502478
> Thanks for this PR :)
>
> Fyi, I pulled in this branch to update delta-rs (see [delta-io/delta-rs#2363](https://github.com/delta-io/delta-rs/issues/2363)). When `gil-refs` is enabled it compiles successfully, but when `gil-refs` is disabled I get a traits bounds error.
>
> ```
> error[E0277]: the trait bound `&str: PyClass` is not satisfied
> --> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:114:47
> |
> 114 | let expected_module = expected_module.extract::<&str>()?;
> | ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
> |
> = help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
> Cow<'a, [u8]>
> Cow<'a, str>
> &'a [u8]
> = note: required for `&str` to implement `pyo3::FromPyObject<'_>`
> = note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
> note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
> --> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
> |
> 1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
> | ------- required by a bound in this associated function
> 1648 | where
> 1649 | T: FromPyObjectBound<'a, 'py>;
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
>
> error[E0277]: the trait bound `&str: PyClass` is not satisfied
> --> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:116:43
> |
> 116 | let expected_name = expected_name.extract::<&str>()?;
> | ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
> |
> = help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
> Cow<'a, [u8]>
> Cow<'a, str>
> &'a [u8]
> = note: required for `&str` to implement `pyo3::FromPyObject<'_>`
> = note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
> note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
> --> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
> |
> 1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
> | ------- required by a bound in this associated function
> 1648 | where
> 1649 | T: FromPyObjectBound<'a, 'py>;
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
>
> error[E0277]: the trait bound `&str: PyClass` is not satisfied
> --> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:119:41
> |
> 119 | let found_module = found_module.extract::<&str>()?;
> | ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
> |
> = help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
> Cow<'a, [u8]>
> Cow<'a, str>
> &'a [u8]
> = note: required for `&str` to implement `pyo3::FromPyObject<'_>`
> = note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
> note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
> --> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
> |
> 1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
> | ------- required by a bound in this associated function
> 1648 | where
> 1649 | T: FromPyObjectBound<'a, 'py>;
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
>
> error[E0277]: the trait bound `&str: PyClass` is not satisfied
> --> /Users/abhiagarwal/.cargo/git/checkouts/arrow-rs-d4122f2d122601f4/b1f5c49/arrow/src/pyarrow.rs:121:37
> |
> 121 | let found_name = found_name.extract::<&str>()?;
> | ^^^^^^^ the trait `PyClass` is not implemented for `&str`, which is required by `&str: FromPyObjectBound<'_, '_>`
> |
> = help: the following other types implement trait `FromPyObjectBound<'a, 'py>`:
> Cow<'a, [u8]>
> Cow<'a, str>
> &'a [u8]
> = note: required for `&str` to implement `pyo3::FromPyObject<'_>`
> = note: required for `&str` to implement `FromPyObjectBound<'_, '_>`
> note: required by a bound in `pyo3::prelude::PyAnyMethods::extract`
> --> /Users/abhiagarwal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-0.21.0/src/types/any.rs:1649:12
> |
> 1647 | fn extract<'a, T>(&'a self) -> PyResult<T>
> | ------- required by a bound in this associated function
> 1648 | where
> 1649 | T: FromPyObjectBound<'a, 'py>;
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyAnyMethods::extract`
> ```
>
> The `FromPyObject` trait gets disabled without `gil-refs`, leading to the error. ([source](https://github.com/PyO3/pyo3/blob/63ba371db021985e01818694cc0987ee213fd4b9/src/conversions/std/slice.rs#L21-L43)) It should be able to be fixed by changing all the `extract` to `extract_bound` per the migration guide.
Hmm, I'm a bit confused by this, as we don't have the `gil-refs` feature enabled and I don't think it's a default feature enabled by `pyo3` (even if it is, we disable the default-features). And the affected code seems to fall in line with what the migration guide suggests: https://pyo3.rs/v0.21.0/migration.html#deactivating-the-gil-refs-feature
Am I missing something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2030566784
I think maybe wait for @abhiaagarwal to resolve their issue first to prove downstream consumers shouldn't be broken by this change? Just in case something was done wrong with this migration :thinking:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "abhiaagarwal (via GitHub)" <gi...@apache.org>.
abhiaagarwal commented on PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566#issuecomment-2030573386
worst case, delta-rs can keep `gil-refs`. if it ends up being an issue with arrow itself, I can try and fix the issue in a separate PR after merge. the feature will exist for awhile!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update py03 from 0.20 to 0.21 [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #5566:
URL: https://github.com/apache/arrow-rs/pull/5566
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org