-
Notifications
You must be signed in to change notification settings - Fork 61
Conversation
raise TypingError('{} The object must be a pandas.series. Given: {}'.format(_func_name, self)) | ||
|
||
def hpat_pandas_series_size_impl(self): | ||
return self._data.size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good but what happened if _data is None
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I can see playing with Pandas in console: pd.Series() does not store _data = None in internal representation. So _data is always something even if Series is empty.
Also this could be replaced with len(self._data)
.
hpat/tests/test_series.py
Outdated
S = pd.Series([np.nan, 1, 2]) | ||
self.assertEqual(hpat_func(S), test_impl(S)) | ||
|
||
S = pd.Series(['1', '2', '3']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PokhodenkoSA It might be better to make these set of tests as a loop.
I mean:
for input_data in test_input_data:
S = pd.Series(input_data)
where data defined as https://github.com/IntelPython/hpat/blob/e45cc922afb855f0e5d87e30acc01ef6ef2e4002/hpat/tests/test_series.py#L2057
also, it might be better to use pd.testing.assert_series_equal
instead self.assertEqual
because, as I understood, the first one compare full Series
datatype but second compares part of it. Yes, we use diffrent types of assertion in tests but it will be a one of the task for redeveloping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think pd.testing.assert_series_equal
is not applicable here because it compares two Series
but Series.size
returns scalar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use test data in loop - done.
No description provided.