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

Add test on Series.getitem failure #619

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add test on Series.getitem failure #619

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions sdc/tests/test_series.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,22 @@ def test_impl(A):
S = pd.Series(['1', '4', '6', '33', '7'], ['1', '3', '5', '4', '7'], name='A')
pd.testing.assert_series_equal(hpat_func(S), test_impl(S))

@unittest.skip('LLVM ERROR: out of memory')
def test_series_getitem_idx_int_negative_slice(self):
def test_impl(S, start, end):
return S[start:end]

jit_impl = self.jit(test_impl)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would probably be better to use the same name (hpat_func) everywhere.


starts = [0, -2]
ends = [-2, 0]
indices = [[2, 3, 5, 6, 7], ['2', '3', '5', '6', '7'], ['2', '3', '5', '6', '7']]
for start, end, index in zip(starts, ends, indices):
Copy link
Contributor

@kozlov-alexey kozlov-alexey Feb 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This would iterate over zipped sequences but until the end of the shortest one is found, so the last string index in indices won't be tested at all.

S = pd.Series([11, 22, 33, 44, 55], index, name='A')
ref_result = test_impl(S, start, end)
jit_result = jit_impl(S, start, end)
pd.testing.assert_series_equal(jit_result, ref_result)
Copy link
Contributor

@kozlov-alexey kozlov-alexey Feb 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Please use self.subTest context to verify each combination in a separate subtest.
Also I think there's no need to have additional test just for negative slices as they have the same implementation, so it's better to make existing test for int slices, i.e. test_series_getitem_idx_int_slice more generic, e.g:

        slice_args = [0, 2, -2]
        indexes_to_test = {
            'int': [2, 3, 5, 6, 7],
            'float': [2.0, 3, 5, 6, 7],
            'string': ['2', '3', '5', '6', '7']
        }
        for start, end in product(slice_args, slice_args):
            for index in indexes_to_test.values():
                with self.subTest(start=start, stop=end, series_index=index):
                    S = pd.Series([11, 22, 33, 44, 55], index, name='A')
                    result_ref = test_impl(S, start, end)
                    result = jit_impl(S, start, end)
                    pd.testing.assert_series_equal(result, result_ref)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kozlov-alexey as far as I get, this is negative tests for negative slices. If it would be combined with positive slices we would need to skip all tests, instead of skiping specific ones.

Copy link
Contributor

Choose a reason for hiding this comment

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

@AlexanderKalistratov What do you mean by negative tests? That an empty slice appear as a result? If so, that's true. Otherwise, I think it should not matter whether slice has negatives or positives from the test perspective (assuming it's a correct slice).

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kozlov-alexey sounds reasonable. I thought this is some kind of numba limitation (negative slices). But it seems to be our own problem.


def test_series_at(self):
def test_impl(S, key):
return S.at[key]
Expand Down