Add 'spearman' correlation method for dataframe.corr and series.corr

[dominicshanshan]

Closes #6804

Adds 'spearman' correlation method for dataframe.corr

[GPUtester]

Can one of the admins verify this patch?

[isVoid]

ok to test

[isVoid]

Thanks for contributing! Can you add unit tests for this method to match pandas output?

[brandon-b-miller]

Thanks very much for contributing this! In addition to what @isVoid raised, can you make it so that method is a kwarg that defaults to pearson so that we match the pandas API here? https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.corr.html

[github-actions]

This PR has been marked stale due to no recent activity in the past 30d. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be marked rotten if there is no activity in the next 60d.

[dominicshanshan]

Thanks for contributing! Can you add unit tests for this method to match pandas output?

here is the unit test script for DataFrame output, will added series calculation as identical as Series.corr in pandas API in coming days + unit test with Series.

update: been discussed with @beckernick , modified a bit about unit test, re-upgrade the unit test script

unit_test_spearman_modify.zip

[isVoid]

@dominicshanshan The unitest looks great! In cudf, we utilize pytest to organize our unit test process. For existing corr function, the coverage is defined in:

cudf/python/cudf/cudf/tests/test_stats.py

Lines 396 to 402 in c929ba1

	def test_df_corr():
	gdf = randomdata(100, {str(x): float for x in range(50)})
	pdf = gdf.to_pandas()
	got = gdf.corr()
	expected = pdf.corr()
	assert_eq(got, expected)

It's likely you can combine the test_spearman and test_pearson into test_df_corr function by parameterizing the method argument. An example of parameterization is at:

cudf/python/cudf/cudf/tests/test_stats.py

Lines 72 to 73 in c929ba1

	@pytest.mark.parametrize("ddof", range(3))
	def test_series_std(ddof):

where test_series_std will run 3 times with ddof parameterized as 0, 1 and 2.

It's also likely you will need

cudf/python/cudf/cudf/tests/utils.py

Line 69 in 2234554

def assert_eq(left, right, **kwargs):

to compare the result. You can specify check_exact=False for float equality.

[dominicshanshan]

to compare the result. You can specify check_exact=False for float equality.

I tested, it seems if your test datatype is float, it will use assert_almost_equal directly, else will use assert_equal, and don't need pass check_exact=False

will follow you guide and give you pytest tomorrow

[isVoid]

Awesome, and please change the base of this PR to branch-0.19.

[dominicshanshan]

Awesome, and please change the base of this PR to branch-0.19.

please find pytest version of unit test for spearman correlation function, I will also finish this feature in Series and pytest, will change to 0.19 brench
pytest_spearman_corr.zip

[dominicshanshan]

black series.py passed

[dominicshanshan]

check again

[isVoid]

I applied some changes to avoid using external only APIs. The below are some new changes to reduce duplication. I think the PR is pretty close!

[codecov]

Codecov Report

Merging #7141 (3ac1595) into branch-22.04 (4596244) will increase coverage by 0.03%.
The diff coverage is 95.93%.

❗ Current head 3ac1595 differs from pull request most recent head 29e31bf. Consider uploading reports for the commit 29e31bf to get more accurate results

@@               Coverage Diff                @@
##           branch-22.04    #7141      +/-   ##
================================================
+ Coverage         86.13%   86.17%   +0.03%     
================================================
  Files               139      141       +2     
  Lines             22438    22508      +70     
================================================
+ Hits              19328    19397      +69     
- Misses             3110     3111       +1     

Impacted Files	Coverage Δ
python/dask_cudf/dask_cudf/tests/test_accessor.py	98.41% <ø> (ø)
python/cudf/cudf/core/_base_index.py	85.92% <50.00%> (-0.51%)	⬇️
python/cudf/cudf/core/column/decimal.py	91.30% <73.68%> (-1.01%)	⬇️
python/cudf/cudf/core/column/categorical.py	89.63% <84.61%> (-0.29%)	⬇️
python/dask_cudf/dask_cudf/io/text.py	85.29% <85.29%> (ø)
python/cudf/cudf/core/column/string.py	88.91% <94.44%> (+0.64%)	⬆️
python/cudf/cudf/core/dataframe.py	93.54% <95.00%> (-0.04%)	⬇️
python/cudf/cudf/core/column/numerical.py	95.62% <95.83%> (+0.64%)	⬆️
python/cudf/cudf/core/frame.py	91.84% <96.42%> (+0.12%)	⬆️
python/cudf/cudf/_typing.py	94.11% <100.00%> (+0.78%)	⬆️
... and 32 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 621d26f...29e31bf. Read the comment docs.

[dominicshanshan]

I applied some changes to avoid using external only APIs. The below are some new changes to reduce duplication. I think the PR is pretty close!

@isVoid , appreciate your help!! why suggest to avoid use external only APIs?

[isVoid]

@isVoid , appreciate your help!! why suggest to avoid use external only APIs?

Outside of the context of this PR, there are many use cases where we only want the column names and do some work with it. Internally cuDF stores column names as keys of the ColumnAccessor data structure. Constructing a pandas index often are just to mimic columns attribute of a pandas dataframe and is often unnecessary work. This work marks this usage in cuDF apis as an anti-pattern that we will check in tests.

In this PR, however, it's sort of an corner case that we do actually want to explicitly construct an index from the column names. Even so, we would use the to_pandas_index method to construct that so the developer's intent is more clearly expressed.

For more about ColumnAccessor, see: https://docs.rapids.ai/api/cudf/stable/basics/internals.html#columnaccessor

[isVoid]

Thanks! Just one more comment that you can feel free to address at your own discretion.

[brandon-b-miller]

LGTM! great job and thanks!

[bdice]

@gpucibot merge