Skip to content

[BEAM-3141][WIP] Support the coders in Python 3#4078

Closed
holdenk wants to merge 27 commits into
apache:masterfrom
holdenk:BEAM-3141-coder-py3-support-r2
Closed

[BEAM-3141][WIP] Support the coders in Python 3#4078
holdenk wants to merge 27 commits into
apache:masterfrom
holdenk:BEAM-3141-coder-py3-support-r2

Conversation

@holdenk

@holdenk holdenk commented Nov 5, 2017

Copy link
Copy Markdown
Contributor

This adds support for the coders in Python 3 as well as fixes some other issues that were blocking GameStatsTest/

@holdenk holdenk force-pushed the BEAM-3141-coder-py3-support-r2 branch from 669c3f9 to 220857d Compare November 5, 2017 07:11
@asfgit

asfgit commented Nov 14, 2017

Copy link
Copy Markdown

SUCCESS

--none--

@asfgit

asfgit commented Nov 14, 2017

Copy link
Copy Markdown

FAILURE

--none--

@asfgit

asfgit commented Nov 14, 2017

Copy link
Copy Markdown

SUCCESS

--none--

@holdenk holdenk force-pushed the BEAM-3141-coder-py3-support-r2 branch 2 times, most recently from 580ba38 to 76917c1 Compare December 29, 2017 02:35
@jkff

jkff commented Feb 7, 2018

Copy link
Copy Markdown
Contributor

What are the next steps for this PR - is it ready for review? If so please specify reviewers.

vlist = [self.decode_from_stream(stream, True) for _ in range(vlen)]
if t == LIST_TYPE:
return vlist
return list(vlist)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So explicitly converts the generator into a list for Py3.

stream.write_byte(FLOAT_TYPE)
stream.write_bigendian_double(value)
elif t is str:
elif t is bytes:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is bytes available in Python 2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yup, it's the same as string in Python 2 :)

span_micros = value.end.micros - value.start.micros
out.write_bigendian_uint64(self._from_normal_time(value.end.micros / 1000))
out.write_var_int64(span_micros / 1000)
out.write_bigendian_uint64(self._from_normal_time(old_div(value.end.micros, 1000)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Prefer // to a function call.

@holdenk holdenk force-pushed the BEAM-3141-coder-py3-support-r2 branch from 76917c1 to 3dcab1e Compare February 12, 2018 07:36
holdenk and others added 20 commits February 15, 2018 05:28
…ame.game_stats_test.GameStatsTest, fix some rethrows. lambda expansion, imports, and metaclass
…GameStatsTest to run, fix serialized values map over, switch some exception re-throws, iteritems, and handle peaking at the eval context.
@holdenk holdenk force-pushed the BEAM-3141-coder-py3-support-r2 branch from 21738e4 to 1bb0328 Compare February 15, 2018 13:31
@holdenk

holdenk commented Feb 15, 2018

Copy link
Copy Markdown
Contributor Author

ping @robertwb or @aaltay ? This doesn't fix everything but it gets at least a super basic pipeline working in Py3

@holdenk

holdenk commented Feb 17, 2018

Copy link
Copy Markdown
Contributor Author

Gentle west-coast daylight hour ping :)

@aaltay

aaltay commented Feb 22, 2018

Copy link
Copy Markdown
Member

@holdenk I was assuming that this was still a WIP PR and not paying much attention in my review list because of the PR title. Other reviewers might be doing that too. I will try to review it this or next week.

@holdenk

holdenk commented Feb 23, 2018

Copy link
Copy Markdown
Contributor Author

Closing in favor of #4730

@holdenk holdenk closed this Feb 23, 2018
@holdenk

holdenk commented Feb 23, 2018

Copy link
Copy Markdown
Contributor Author

Note: once the dust settles I may cherry-pick some parts of this forward into a new coder related PR but keeping the large change up-to-date when we've switched to using six seems not overly useful.

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.

5 participants