Fix Bug where _wrap handles Pandas DFs incorrectly #122
Conversation
|
Thanks for the PR! I took a look at the failed tests, and they look unrelated to the PR (version of lint checker upgraded and code hasn't been changed to adapt yet). Things look good to me with the exception of a cleaner way of skipping the test if pandas isn't installed which I'll mention in a sec. Once that is changed then I'll merge, and fix those random lint errors. |
functional/test/test_functional.py
Outdated
| self.assertEqual(result[1].to_list(), ['name2', 2]) | ||
| self.assertEqual(result[2].to_list(), ['name1', 3]) | ||
| self.assertEqual(result[3].to_list(), ['name2', 4]) | ||
| except ImportError: |
There was a problem hiding this comment.
You can use builtin skip test ability like here https://github.com/EntilZha/PyFunctional/blob/master/functional/test/test_util.py#L23. I would make a helper function like pandas_is_installed() and call that.
functional/test/test_functional.py
Outdated
| @@ -800,6 +800,21 @@ class A(object): | |||
| self.assertIsInstance(_wrap(A()), A) | |||
| self.assert_type(self.seq(l)) | |||
|
|
|||
| def test_wrap_pandas(self): | |||
| # pylint: disable=superfluous-parens | |||
There was a problem hiding this comment.
Not sure why this would be correctly throwing a lint error. Would you mind removing it, and if it turns out to be a lint error I disagree with I'll re-insert.
Codecov Report
@@ Coverage Diff @@
## master #122 +/- ##
==========================================
- Coverage 99.9% 98.47% -1.43%
==========================================
Files 12 12
Lines 2149 2169 +20
==========================================
- Hits 2147 2136 -11
- Misses 2 33 +31
Continue to review full report at Codecov.
|
|
I've used a similar decorator to the one you suggested 👍 |
|
Thanks for the fix! Great work, LGTM to merge |
* Fix Bug where _wrap handles Pandas DFs incorrectly * remove pylint ignore & clean pandas install check
Closes #97