Skip to content
This repository has been archived by the owner on Feb 2, 2024. It is now read-only.

Implement Series.mean() in new style #219

Merged
merged 2 commits into from
Oct 20, 2019

Conversation

PokhodenkoSA
Copy link
Contributor

No description provided.

hpat/datatypes/hpat_pandas_series_functions.py Outdated Show resolved Hide resolved
hpat/tests/test_series.py Outdated Show resolved Hide resolved
hpat/tests/test_series.py Outdated Show resolved Hide resolved
hpat/tests/test_series.py Outdated Show resolved Hide resolved
@@ -1074,6 +1074,67 @@ def hpat_pandas_series_max_impl(self, axis=None, skipna=True, level=None, numeri
return hpat_pandas_series_max_impl


@overload_method(SeriesType, 'mean')
def hpat_pandas_series_mean(self, axis=None, skipna=None, level=None, numeric_only=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def hpat_pandas_series_mean(self, axis=None, skipna=None, level=None, numeric_only=None):
def hpat_pandas_series_mean(self, axis=None, skipna=True, level=None, numeric_only=None):

'{} Unsupported parameters. Given axis: {}, level: {}, numeric_only: {}'.format(_func_name, axis, level,
numeric_only))

def hpat_pandas_series_mean_impl(self, axis=None, skipna=None, level=None, numeric_only=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def hpat_pandas_series_mean_impl(self, axis=None, skipna=None, level=None, numeric_only=None):
def hpat_pandas_series_mean_impl(self, axis=None, skipna=True, level=None, numeric_only=None):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shssf could you please explain in this issue #233 for me and all why we should replace Pandas signature skipna=None with our skipna=True?
I really do not understand which criteria I should use to create signature for our functions, which default values I should replace and which to remain. My first suggestion was - do like in Pandas.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PokhodenkoSA I would refer to Pandas sources. Please follow signature defined there. Documentation might have a bugs as a regular code. Anyway, by the algo you implemented it is True despite of the fact it None

hpat/tests/test_series.py Outdated Show resolved Hide resolved
else:
self.assertEqual(actual, expected)

@unittest.skipIf(hpat.config.config_pipeline_hpat_default, "Series.mean() any parameters unsupported")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if hpat.config.config_pipeline_hpat_default is True it has no parameters support?
This parameter is True by default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is correct. This test does not work with HPAT pipeline.

hpat/tests/test_series.py Outdated Show resolved Hide resolved
Remove mean from HPAD pipeline and add tests
Removed commented code
Fix tests
Rename tests for mean() and test both skipna cases
Combine all tests for mean() in one
Fix Test list in docstring for mean() implementation
Fix for complex numbers and comment mean in series_kernels.py
Return mean to series_replace_funcs because it is used in other places
Fix docstring for mean
Return back mean in _run_call_series and add test for default skipna for old style
Improve tests
'{} Unsupported parameters. Given axis: {}, level: {}, numeric_only: {}'.format(_func_name, axis, level,
numeric_only))

def hpat_pandas_series_mean_impl(self, axis=None, skipna=None, level=None, numeric_only=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PokhodenkoSA I would refer to Pandas sources. Please follow signature defined there. Documentation might have a bugs as a regular code. Anyway, by the algo you implemented it is True despite of the fact it None

@shssf shssf merged commit b1b281f into IntelPython:master Oct 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants