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

Conversation

@kozlov-alexey
Copy link
Contributor

No description provided.

@pep8speaks
Copy link

pep8speaks commented Jun 10, 2020

Hello @kozlov-alexey! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-06-16 10:57:54 UTC

@kozlov-alexey kozlov-alexey force-pushed the bugfix/comp_binops_fill_value branch from 6433e1a to 5ea6970 Compare June 10, 2020 09:56
@kozlov-alexey
Copy link
Contributor Author

Tested full series suite on python 3.6. and with this fix it passed:

Ran 488 tests in 2085.784s
OK (skipped=131, expected failures=14)

with self.subTest(data=case):
np.testing.assert_array_equal(ref_impl(array0), sdc_func(array1))

def test_fillna_numeric_inplace_false(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

You could combine tests test_fillna_numeric_inplace_false and test_fillna_numeric_inplace_true by moving generic code to separate method something like that:

def _test_fillna_numeric_inplace(self, pyfunc, cfunc):
    # generic code

Then call the method in the tail of both tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@densmirn The assertion code block of these tests are not identical (inplace=True methods modify original arrays), so this new _test_fillna_numeric func has to accept inplace arg and alter behavior based on it's value (made it this way).

@AlexanderKalistratov AlexanderKalistratov merged commit b674bb1 into IntelPython:integration/release_0.33.0 Jun 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants