add Series.corr/3 and Series.cov/2

[anthony-khong]

This is based on corr and cov. For corr, I did not implement the spearman_rank_corr function, because it needs the propagate_nans Polars feature. I wonder what's the appetite for adding new Polars features? Should we do it in this case?

[philss]

Awesome! 🚢

I think we could discuss the naming, but the features looks good to me!

[philss]

I would call it correlation and covariance, since they are short names and we usually prefer to avoid abbreviations. But let's hear what José thinks :)

[josevalim]

Agreed!

[anthony-khong]

Thank you for the feedback! I've changed the names to correlation and covariance 👍

[anthony-khong]

Oh, there was one issue I ran into when developing this. I don't think it's related to the features here, but I may as well report it before this gets merged.

This works:

DF.new(a: [1.0, 8.0, 3.0, nil], b: [4.0, 5.0, 2.0, nil])
|> DF.mutate(c: covariance(a, b), d: correlation(a, b))

# #Explorer.DataFrame<
#  Polars[4 x 4]
#  a float [1.0, 8.0, 3.0, nil]
#  b float [4.0, 5.0, 2.0, nil]
#  c float [3.0, 3.0, 3.0, 3.0]
#  d float [0.5447047794019223, 0.5447047794019223, 0.5447047794019223,
#   0.5447047794019223]
# >

but this panics:

DF.new(a: [1.0, 8.0, 3.0], b: [4.0, 5.0, 2.0])
|> DF.concat_rows(DF.new(a: [nil], b: [nil]))
|> DF.mutate(c: covariance(a, b), d: correlation(a, b))

# thread '<unnamed>' panicked at 'validity must be equal to the array's length', [...]/.cargo/registry/src/index.crates.io-6f17d22bba15001f/arrow2-0.17.2/src/array/primitive/mod.rs:241:5
# ** (ErlangError) Erlang error: :nif_panicked
# ...

I would've thought that DF.new(a: [1.0, 8.0, 3.0, nil], b: [4.0, 5.0, 2.0, nil]) is equivalent to DF.new(a: [1.0, 8.0, 3.0], b: [4.0, 5.0, 2.0]) |> DF.concat_rows(DF.new(a: [nil], b: [nil]))? Is this a bug, or perhaps I'm understanding concat_rows wrong?

[josevalim]

You are using it correctly. This looks like a polars bug. It is failing to merge two dataframes when one has nils and the other does not. Can you isolate it on the Rust side?

[josevalim]

💚 💙 💜 💛 ❤️

[anthony-khong]

You are using it correctly. This looks like a polars bug. It is failing to merge two dataframes when one has nils and the other does not. Can you isolate it on the Rust side?

Honestly, I don't know enough about Polars to debug this, but changing the following stops the panic:

diff --git a/native/explorer/src/dataframe.rs b/native/explorer/src/dataframe.rs
index fff57c5..0fe9389 100644
--- a/native/explorer/src/dataframe.rs
+++ b/native/explorer/src/dataframe.rs
@@ -107,7 +107,7 @@ pub fn df_concat_rows(
         out_df.vstack_mut(&df)?;
     }
     // Follows recommendation from docs and rechunk after many vstacks.
-    out_df.rechunk();
+    out_df.as_single_chunk_par();
     Ok(ExDataFrame::new(out_df))
 }

I think it's something to do with the should_rechunk function, seeing that the rechunk function is defined as:

    pub fn rechunk(&mut self) -> &mut Self {
        if self.should_rechunk() {
            self.as_single_chunk_par()
        } else {
            self
        }
    }

What do you think? Would it be worth fixing the bug by skipping their should_rechunk function and rechunk every time concat_rows is called?

[josevalim]

I think so. By definition there is more than one chunk, so we can always force it.