build: Revert PyO3 version back to 0.21

[stinodego]

Partially reverts #19199

At the request of Ritchie. Apparently this should solve a deadlock.

@ritchie46 is a full revert needed or just the version rollback?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.17%. Comparing base (27289b2) to head (5c56c0e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #19376      +/-   ##
==========================================
- Coverage   80.18%   80.17%   -0.01%     
==========================================
  Files        1523     1523              
  Lines      209897   209803      -94     
  Branches     2434     2434              
==========================================
- Hits       168314   168219      -95     
- Misses      41028    41029       +1     
  Partials      555      555              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ritchie46]

@stinodego we need to revert the Clone implementations

[itamarst]

I'm poking around to see if there's a specific one that can be reverted, rather than all.

[itamarst]

Since this is motivated by #19358, the thing to do is add add the reproducer as a test and then you can tell if it's sufficient. It doesn't solve it, alas. You probably need to revert all of #19199 (unless I can come up with a quick fix ASAP).

[ritchie46]

@itamarst it is ObjectValue. The rest can be put behind an Arc.

Taking the gil in a Clone is unacceptable for Polars as that means we can always end up in a deadlock if we parallelize. It can we always release the GIL when going into Polars which is good behavior. Let's do that refactor.

[itamarst]

As mentioned in the issue, I think PyDataFrame and friends should be releasing the GIL much more aggressively. I've fixed a few other cases where you got deadlocks, unrelated to cloning, because GIL wasn't released when waiting on Rayon.

Here's a patch that fixes this particular reproducer (but is unlikely to fix all potential regressions):

--- a/crates/polars-python/src/dataframe/general.rs
+++ b/crates/polars-python/src/dataframe/general.rs
@@ -269,10 +269,11 @@ impl PyDataFrame {
         Ok(PyDataFrame::new(df))
     }
 
-    pub fn gather_with_series(&self, indices: &PySeries) -> PyResult<Self> {
+    pub fn gather_with_series(&self, py: Python, indices: &PySeries) -> PyResult<Self> {
         let indices = indices.series.idx().map_err(PyPolarsErr::from)?;
-        let df = self.df.take(indices).map_err(PyPolarsErr::from)?;
+        let df = Python::allow_threads(py, || self.df.take(indices).map_err(PyPolarsErr::from))?;
         Ok(PyDataFrame::new(df))
+
     }
 
     pub fn replace(&mut self, column: &str, new_col: PySeries) -> PyResult<()> {

[ritchie46]

@itamarst do you make a new PR for that?

[itamarst]

Narrowly just that method (and any others you think might be relevant to this particular regression) I can probably do in the next couple hours.

[ritchie46]

If we do that and temporarily rollback the pyo3 version and clone implementations, we are safe and have a bit more time to move all methods over afther we pathc.

[itamarst]

OK, I'll do the GIL bit (and a test) and I'll let someone else do the rollback since I'm not sure which is the correct git procedure.

[itamarst]

#19383 is the narrow GIL release PR.

[stinodego]

@stinodego we need to revert the Clone implementations

Done. Let me know if there is more that also needs to be reverted.