OEP-7: Python 3 migration plan #21

jcdyer · 2016-09-07T19:01:07Z

Plan for migrating Open edX code to Python 3.

clintonb · 2016-09-23T21:49:28Z

oeps/oep-0007.rst

+With primary development of Python occurring on Python 3, and with Python 2
+scheduled for end-of-life in 2020, edX needs to plan for a transition of all of
+our code to Python 3.  Open edX is a large project, spanning many applications
+over even more github repos, with even more dependencies on third party


s/github repos/repositories

clintonb · 2016-09-23T21:49:48Z

oeps/oep-0007.rst

+our code to Python 3.  Open edX is a large project, spanning many applications
+over even more github repos, with even more dependencies on third party
+libraries.  This document outlines how we plan to scope the problem, socialize
+understanding of the differences between versions of python, migrate our code,


Capitalize Python

clintonb · 2016-09-23T21:50:22Z

oeps/oep-0007.rst

+Rationale
+=========
+
+    * Python 2 will only receive support from the python core devs until 2020.


Capitalize Python.

jcdyer · 2016-10-05T20:44:31Z

oeps/oep-0007.rst

+
+
+Choosing a python version
+++++++++++++++++++++++++


Why polyglot?

nedbat · 2016-10-05T20:44:55Z

oeps/oep-0007.rst

+OEP-0007: Migrating to Python 3
+===============================
+---------------+-------------------------------------------+
+| OEP           | :doc:`OEP-7 </oeps/oep-0007>`             |


When you rebase onto master, you'll need to change this to <oep-0007>.

jcdyer · 2016-10-05T20:47:24Z

oeps/oep-0007.rst

+Choosing a python version
+++++++++++++++++++++++++
+
+All greenfield python projects should be written using python 3 unless a


s/greenfield/new/

jcdyer · 2016-10-05T20:48:28Z

oeps/oep-0007.rst

+solution, edX will maintain a repository of compatibility shims (edx-compat?).
+Ideally, all edx-maintained code that implements different behavior based on
+python version will be in this repo.
+


Packaging: Use proper trove classifiers.

maxrothman · 2016-11-10T20:21:07Z

oeps/oep-0007.rst

+
+All files should have the main `__future__` imports at the top to regularize
+some behaviors that differ by default between Python 2 and 3. `from
+__future__ import absolute_import` forbid the use of implicit relative


Nit: these would be more readable as a list

maxrothman · 2016-11-10T20:21:41Z

oeps/oep-0007.rst

+
+Migrating code
++++++++++++++
+


We should have links to six and future's documentation somewhere in this document

maxrothman · 2016-11-10T20:24:59Z

oeps/oep-0007.rst

+...........................................
+
+In order to write code that looks as much like native Python 3 as possible,
+you may want to use `from __future__ import unicode_literals`, which makes bare


this does not make code formatting, it makes quote formatting

maxrothman · 2016-11-10T20:29:58Z

oeps/oep-0007.rst

+work to change after the fact.  This can be avoided in some cases by iterating
+directly over the dict object.  Instead of using:
+
+    for key, value in six.viewitems(d):


These format as quote blocks, not code blocks

maxrothman · 2016-11-10T20:30:52Z

oeps/oep-0007.rst

+--------------
+
+If you find other incompatibilities, a shim will likely be found in [the `six`
+library](https://pythonhosted.org/six/).  For incompatibilies with no other


Ah, here's the link. It probably should've been up at the first reference.

maxrothman · 2016-11-10T20:31:28Z

oeps/oep-0007.rst

+arise during migration.
+
+Appropriate tooling will help.  Tests should be configured to run under both
+Python 2 or 3 (tox helps with this).  As it would double the cost of running


link to tox

jmbowman · 2016-11-10T22:07:06Z

oeps/oep-0007.rst

+Before converting a codebase to Python 3, we need to make sure the code we
+depend on will also support Python 3.  We can make a rough check for
+third-party library support using the [Can I Use Python 3 website or
+library](https://caniusepython3.com/).  This will sometimes show inaccurate


This link and 5 others below use Markdown formatting, but this is a reStructuredText document. They need to be updated to render correctly in the output.

jmbowman · 2016-11-10T22:10:10Z

oeps/oep-0007.rst

+possible, we will use unambiguous text or byte objects.  In most cases, text
+should be preferred.  Bytes should only be used when you can answer the
+question: "Do I need this specific sequence of bytes."  The most
+error-resistent way to acheive this is to use what is called a "unicode


Spelling: resistent -> resistant

jmbowman · 2016-11-10T22:11:48Z

oeps/oep-0007.rst

+bytes-oriented library). The only operation that should (ideally) be performed
+on bytes is decoding.
+
+In those cases where ambiguity is required (such as working with libraries like


The matching close parenthesis is missing

jmbowman · 2016-11-10T22:19:45Z

oeps/oep-0007.rst

+developers working on that project.
+
+Handling literals, Option 1: Python 3-Style
+...........................................


Periods don't seem to be among the valid section header underline characters

jmbowman · 2016-11-10T22:22:07Z

oeps/oep-0007.rst

+
+    native = 'string'  # pylint: disable=native-string
+
+This version creates noisier code than Option 2, below, but makes it easier to


This is Option 2; did you mean to reference Option 1 above?

jmbowman · 2016-11-10T22:28:47Z

oeps/oep-0007.rst

+    Programming Language :: Python :: 2
+    Programming Language :: Python :: 2 :: Only
+    Programming Language :: Python :: 3
+    Programming Language :: Python :: 3 :: Only


The odd-numbered lines from these blocks are being omitted from the output; the colons are probably throwing off the formatting somehow

Shows up now. I'm not sure if I changed something or not.

jmbowman · 2016-11-10T22:32:44Z

oeps/oep-0007.rst

+Appropriate tooling will help.  Tests should be configured to run under both
+Python 2 or 3 (tox helps with this).  As it would double the cost of running
+tests to run both all the time, we may only want to enable Python 3 tests in
+the CI environment for repos that are actively being migrated.  We still want


The extra test runs shouldn't matter for repos that use Travis, as they're run in parallel on external hardware and don't incur usage fees.

They will eat into our available travis workers, which we're already a little short on, leading to more queue backup and longer delays for tests to run.

jmbowman · 2016-11-10T22:35:01Z

oeps/oep-0007.rst

+  * Deploy xblocks separately to test remote execution
+  * Migrate to Python 3
+  * Upgrade external xblocks as needed, and support partners who wish to do the
+    same


This isn't valid rst formatting for nested lists; the type of bullet needs to change for the 2nd level

jmbowman · 2016-11-10T22:36:39Z

oeps/oep-0007.rst

+Deploying Services
++++++++++++++++++
+
+Once a service has been achieved full Python 3 compatibility, we will need to


Grammar, probably just need to drop "been"

bmedx · 2016-12-14T18:58:51Z

oeps/oep-0007.rst

+where each one is preferable.  The two options are `six`, and `future`.
+
+Six is a simple compatibility library.  If something works differently in
+Python 2 and and Python 3, you import six, and use functions that it provides


jcdyer · 2017-01-17T19:19:59Z

@jmbowman: I think I've responded to all your concerns. Please take another look.

jmbowman

One last formatting nitpick, other than that looks good.

jmbowman · 2017-01-17T20:31:08Z

oeps/oep-0007.rst

+
+* IDAs that we want to continue supporting in the future
+  + Old IDAs (that we want to replace) should not be upgraded, but we will need
+    to prioritize replacement to occur during the migration timeframe.


The nested bullets here and in the third bullet point need to be separated by blank lines from the parent list items to be formatted correctly

benpatterson · 2017-01-19T02:34:13Z

oeps/oep-0007.rst

+Appropriate tooling will help.  Tests should be configured to run under both
+Python 2 or 3 using tox.  As it would double the cost of running
+tests to run both all the time, we may only want to enable Python 3 tests in
+the CI environment for repos that are actively being migrated.  We still want


Please don't worry about this. We are building our CI infrastructure for scale. If we reach a point where we can't support what we need to do, then we can think about removing pieces...

Also, technically...due to our billing, prices won't be doubled by using more builds in Travis, for example. So the statement is inaccurate. I argue that you remove this value statement altogether; we should just test what we need to test.

Thanks, Ben. That's helpful information.

mulby · 2017-01-19T18:01:12Z

oeps/oep-0007.rst

+
+
+New libraries should additionally support Python 2 using a single code-base
+with Six_ as a compatibility layer if they will be used by projects that run on


We have, at times, struggled with different libraries requiring different versions of six.

mulby · 2017-01-19T18:15:07Z

oeps/oep-0007.rst

+can use ``futurize``, and what other work needs to be done.
+
+
+Deploying Services


I think you could remove this section. This is true of all significant changes like this and only applicable to edx.org

mulby · 2017-01-19T18:17:22Z

oeps/oep-0007.rst

+---------------+-------------------------------------------+
+| Author        | Cliff Dyer <cdyer@edx.org>                |
+---------------+-------------------------------------------+
+| Arbiter       |                                           |


Don't forget to update this with the actual arbiter (not sure who it is for this OEP).

blarghmatey · 2017-01-19T20:15:05Z

oeps/oep-0007.rst

+  statement.
+* ``from __future__ import division`` will make single-slash division
+  (``a / b``) always perform floating point division, and double-slash division
+  (``a // b``).


Incomplete sentence. Should read and double slash division ("a // b") perform integer division

pdpinch · 2017-01-20T16:12:02Z

Since course content can contain python code, I think you need a section that addresses it specifically.

I presume the python version is controlled by the sandbox setting. But I think there's a feature flag for disabling the sandbox, in which case I think python in courseware would use the same version that edx-platform is running on.

jcdyer · 2017-01-20T20:35:44Z

Great idea @pdpinch. I'll add that in.

pdpinch · 2017-01-20T22:01:27Z

credit should go to @Colin-Fredericks for calling it out -- I'm just a (very) interested party

jcdyer · 2017-01-20T22:04:07Z

Thanks @Colin-Fredericks! I've added language to address that in the "Order of Migrations" section, under "edx-platform". (Also, I think we went to high school together.)

Colin-Fredericks · 2017-01-23T18:48:15Z

@jcdyer Huh! So we did! Now I need to dig out my old 1996 NMH Facebook and find your picture. Thanks for the help here.

jcdyer · 2017-01-23T19:33:11Z

@Colin-Fredericks Think big hair.

@jmbowman I believe this OEP is ready to merge.

jmbowman

Looks good to go! Just update the Status and Resolution fields accordingly.

jcdyer force-pushed the cdyer/oep-0007-python3-migration branch from 2c667bb to a65f2cd Compare September 23, 2016 17:43

clintonb reviewed Sep 23, 2016

View reviewed changes

jcdyer commented Oct 5, 2016

View reviewed changes

oeps/oep-0007.rst

Choosing a python version

+++++++++++++++++++++++++

Copy link

Author

jcdyer Oct 5, 2016

Choose a reason for hiding this comment

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

Why polyglot?

nedbat reviewed Oct 5, 2016

View reviewed changes

jcdyer commented Oct 5, 2016

View reviewed changes

jcdyer changed the title ~~[WIP] OEP-7: python3 migration plan~~ OEP-7: python3 migration plan Oct 27, 2016

maxrothman reviewed Nov 10, 2016

View reviewed changes

jmbowman suggested changes Nov 10, 2016

View reviewed changes

jcdyer changed the title ~~OEP-7: python3 migration plan~~ OEP-7: Python 3 migration plan Nov 18, 2016

bmedx reviewed Dec 14, 2016

View reviewed changes

jmbowman approved these changes Jan 17, 2017

View reviewed changes

jcdyer force-pushed the cdyer/oep-0007-python3-migration branch 2 times, most recently from b834e4e to 696b257 Compare January 17, 2017 21:10

benpatterson reviewed Jan 19, 2017

View reviewed changes

mulby reviewed Jan 19, 2017

View reviewed changes

mulby approved these changes Jan 19, 2017

View reviewed changes

blarghmatey reviewed Jan 19, 2017

View reviewed changes

jcdyer force-pushed the cdyer/oep-0007-python3-migration branch from 9da45bd to 9211365 Compare January 20, 2017 22:02

jcdyer force-pushed the cdyer/oep-0007-python3-migration branch from 9211365 to 85563ae Compare January 23, 2017 20:26

OEP-7: Migrating to Python 3

3e170cc

jcdyer force-pushed the cdyer/oep-0007-python3-migration branch from 85563ae to 3e170cc Compare January 23, 2017 20:27

jmbowman approved these changes Jan 23, 2017

View reviewed changes

Mark OEP Accepted and set resolution field.

caaf131

jcdyer merged commit a0b9a92 into master Jan 23, 2017

feanil deleted the cdyer/oep-0007-python3-migration branch May 23, 2019 15:57


		native = 'string' # pylint: disable=native-string

		This version creates noisier code than Option 2, below, but makes it easier to



		New libraries should additionally support Python 2 using a single code-base
		with Six_ as a compatibility layer if they will be used by projects that run on

		can use ``futurize``, and what other work needs to be done.


		Deploying Services

OEP-7: Python 3 migration plan #21

OEP-7: Python 3 migration plan #21

Conversation

jcdyer commented Sep 7, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcdyer commented Jan 17, 2017

jmbowman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdpinch commented Jan 20, 2017

jcdyer commented Jan 20, 2017

pdpinch commented Jan 20, 2017

jcdyer commented Jan 20, 2017 • edited

Colin-Fredericks commented Jan 23, 2017

jcdyer commented Jan 23, 2017

jmbowman left a comment

Choose a reason for hiding this comment

jcdyer commented Sep 7, 2016 •

edited

jcdyer commented Jan 20, 2017 •

edited