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

Conversation

PokhodenkoSA
Copy link
Contributor

No description provided.

if not isinstance(self, SeriesType):
raise TypingError('{} The object must be a pandas.series. Given: {}'.format(_func_name, self))

if not isinstance(self.data.dtype, (types.Integer, types.Float)):
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use self.dtype instead of self.data.dtype, but perhaps it's also ok.

Copy link
Contributor

@shssf shssf left a comment

Choose a reason for hiding this comment

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

skipna default is wrong or needs to be checked with Pandas sources

# TODO: ignore non-numerics
# get series prod output types
dtypes = tuple(hpat.hiframes.pd_series_ext.SeriesAttribute.resolve_prod(
dtypes = tuple(numba.typing.arraydecl.ArrayAttribute.resolve_prod(
Copy link
Contributor

Choose a reason for hiding this comment

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

What issue you trying to solve by this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SeriesAttribute have no resolve_prod() because it was not copied from ArrayAttribute.
Here I added 'resolve_prod' into _not_series_array_attrs and it prevents copying of corresponding method from ArrayAttribute to SeriesAttribute here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is exception with 'attribute not found'



@overload_method(SeriesType, 'prod')
def hpat_pandas_series_prod(self, axis=None, skipna=None, level=None, numeric_only=None, min_count=0):
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_prod(self, axis=None, skipna=None, level=None, numeric_only=None, min_count=0):
def hpat_pandas_series_prod(self, axis=None, skipna=True, level=None, numeric_only=None, min_count=0):

According to documentation:
skipna : bool, default True

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None in Python is usually used as default value because Python consider default values as global values (see this).
Example:

def f(a=[]):
  a.append(1)
  return a

>>> f()
[1]
>>> f()
[1,1]

So common approach is:

def f(a=None):
   """Docstring: a: list, default []"""
  if a is None:
    a = []
  ...

Copy link
Contributor Author

@PokhodenkoSA PokhodenkoSA Oct 18, 2019

Choose a reason for hiding this comment

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

In this case with bool True is global and not mutable. But implementation of Pandas takes None as argument. Moreover Pandas is ready to get None so we also should be ready to get None and set it to True in the function.

So I think skipna : bool, default True is about logical value. It meas if you drop out skipna it will be considered as True.
Example:

S.prod()
S.prod(True)

Both calls are equivalent.

skipna: :obj:`bool` object, default :obj:`True`
self: :obj:`pandas.Series`
input series
axis: :obj:`int`
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually axis could be a string ('index' or 'columns'), not only an integer. You could add string type here. Moreover you could check on typing stage of the implementation that axis type is able to be a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created #235 for discussing documentation for parameters.

PokhodenkoSA and others added 3 commits October 18, 2019 10:53
Improve exception message for min_count value for Series.prod()
Improve test
Add tests for skipna=None as default
Fix prod(skipna=None) and use subTest() for better error indication when testing on a set of data
Describe the parameters and add types to docstring
Use SeriesType().dtype instead of SeriesType.data.dtype
Use numba.typing.arraydecl.ArrayAttribute.resolve_prod() instead of hpat.hiframes.pd_series_ext.SeriesAttribute.resolve_prod() because SeriesAttribute does not have resolve_prod
Update docstring for hpat_pandas_series_prod
Fix docstring
@shssf shssf merged commit 6ed2abe 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.

3 participants