Numpy bool msgpack bugfix #18395
Numpy bool msgpack bugfix #18395
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18395 +/- ##
==========================================
- Coverage 91.35% 91.34% -0.02%
==========================================
Files 163 163
Lines 49691 49691
==========================================
- Hits 45397 45388 -9
- Misses 4294 4303 +9
Continue to review full report at Codecov.
|
pandas/io/msgpack/_packer.pyx
Outdated
| @@ -133,7 +134,7 @@ cdef class Packer(object): | |||
| while True: | |||
| if o is None: | |||
| ret = msgpack_pack_nil(&self.pk) | |||
| elif isinstance(o, bool): | |||
| elif isinstance(o, (bool, bool_)): | |||
pandas/tests/io/msgpack/test_pack.py
Outdated
| @@ -25,6 +26,7 @@ def testPack(self): | |||
| (), ((),), ((), None,), | |||
| {None: 0}, | |||
| (1 << 23), | |||
There was a problem hiding this comment.
can you add a round-trip test for the example in the issue itself.
There was a problem hiding this comment.
This test should already test for the buggy behavior when it comes to np.bool_.
Should I revert the changes to this test and add a one checking round-trip behavior or should I make that test a additional one?
Should the test for the to_msgpack() round-trip behavior or packer roundtrip behavior?
If it should check for to_msgpack() behavior where would I best add this test? (There is no /test/io/formats/test_to_msgpack.py)
|
I applied the requested changes. |
|
can you add the tests from the original issue as well, e.g. the round-trip of the object array containing bools. rebase and ping on green. |
e82439a to
dd73128
Compare
|
I added the requested check. |
|
lgtm. just needs a rebase. ping when pushed. |
This allows for running to_msgpack with dataframes containing field of numpy.bool_ datatype
Added test for scaler numpy bool, added numpy bool to mixed list test, removed modifications to pack test and changend bool_ to np.bool_
dd73128 to
378a218
Compare
|
rebased. |
|
thanks @PhyNerd |
(cherry picked from commit 6c074d1)
(cherry picked from commit 6c074d1)
This fixes issue #18390.
It allows for running DataFrame.to_msgpack() with dataframes containing fields of the numpy.bool_ datatype.
git diff upstream/master -u -- "*.py" | flake8 --diff