Added 'pearson' to methods list in pandas/core/nanops.py #30603
Conversation
ShaharNaveh
changed the title
ci/deps/azure-macos-36.yaml
Outdated
| @@ -26,6 +26,7 @@ dependencies: | |||
| - pytables | |||
| - python-dateutil==2.6.1 | |||
| - pytz | |||
| - scipy>=1.1 | |||
There was a problem hiding this comment.
hmm, we should be skipping tests if the deps are not installed, which test is the issue?
There was a problem hiding this comment.
hmm, we should be skipping tests if the deps are not installed, which test is the issue?
pandas.tests.frame.methods.test_cov_corr.TestDataFrameCorrWith
test_moments_rolling.TestRollingMomentsConsistency
pandas.tests.frame.methods.test_cov_corr.TestDataFrameCorrWith
pandas.tests.test_nanops.TestnanopsDataFrame
pandas.tests.test_nanops.TestnanopsDataFrame
pandas.tests.frame.methods.test_cov_corr.TestDataFrameCorrWith
pandas.tests.series.test_timeseries.TestTimeSeries
For the macOS build, at least.
There was a problem hiding this comment.
then these should have a skip in place
pandas/core/nanops.py
Outdated
| @@ -1241,7 +1241,7 @@ def nancorr(a, b, method="pearson", min_periods=None): | |||
|
|
|||
|
|
|||
| def get_corr_func(method): | |||
| if method in ["kendall", "spearman"]: | |||
| if method in ["kendall", "spearman", "pearson"]: | |||
There was a problem hiding this comment.
actrually, why is this necessary at all? this just uses numpy and not scipy
There was a problem hiding this comment.
You asked me to:)
ref - #30461 (comment)
There was a problem hiding this comment.
ahh right, but this is not actually the correct check, instead do like
if method in ['kendall', 'spearman']:
.....
elif method in ['pearson']:
pass
elif callable(method):
....
else:
raise ValueError("....in valid method passed')
There was a problem hiding this comment.
raise ValueError(f"Unrecognized method '{method}'")Sounds good? (probably not, my English is not so good)
And also, should I remove all the "Skip" decorators Iv'e added in ed6f9c9 ?
There was a problem hiding this comment.
sure & yes remove the skips
| elif callable(method): | ||
| return method | ||
| else: | ||
| raise ValueError( | ||
| f"Unkown method '{method}', expected one of 'kendall', 'spearman'" |
There was a problem hiding this comment.
can you add a test that hits this (likely just calling .corr(method='foo') would do it
|
Closed&Reopened because the build failed during compile time. Checking if that was a false positive. |
|
thanks @MomIsBestFriend |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff