Skip to content

Conversation

@devendor
Copy link
Contributor

The original batch wasn't working for jython and seemed to be more a lack of implicit type casting in older objects and alternate implementation pythons. This seems less intrusive, more straightforward and works on jython-2.7.1.

methane and others added 6 commits September 28, 2018 22:18
Python 3.4 is not supported officially.
But keep running test for a while, to know when msgpack-python
stop working on Python 3.4 actually.
casting.

The current patches did not work under jython-2.7.1 where implicit
casting of buffer or memoryview doesn't work. It may also be the
jython is a little pickier about string casting non string bytes
due to the underlying strong typing of java.

See issues msgpack#303 & msgpack#304.

Rolls back / replaces:
aa41e2f
5f684ae
b10cf78
Replace jython and legacy cpython fixes with explicit type casting.
_unpack_from = staticmethod(
struct.unpack_from if sys.version_info >= (2, 7, 6)
else lambda f,b,o=0: struct.unpack_from(f, b[:o+2].tobytes(),o)
)
Copy link
Member

Choose a reason for hiding this comment

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

Use global function instead of staticmethod.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry it took so many tries. I have a head cold so it was like typing three lines of code with my toes this morning.

process(o)
"""

Copy link
Member

Choose a reason for hiding this comment

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

Don't this

self._buf_checkpoint = 0

self._buffer += view
self._buffer += bytearray(view)
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this increase data copy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is an interesting question. My assumption is that the inplace add / concatenation operator is doing an implicit typecast for you in the background rather than throwing TypeError in newer python, so an explicit typecast was just replacing a step that had to happen implicitly somewhere anyway. I don't actually want to read a bunch of code to verify that assumption though. I figured out that .extend() works reliably with implicit typecasting in old and new which should be about an even trade.

@methane methane merged commit 70b5f21 into msgpack:master Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants