QuantEcon / sphinxcontrib-jupyter

A Sphinx Extension for Generating Jupyter Notebooks
BSD 3-Clause "New" or "Revised" License
76 stars 23 forks source link

Python 2 support #62

Closed nthiery closed 6 years ago

nthiery commented 6 years ago

The commits makes some minor changes that enable using sphinxcontrib-jupyter with Python 2. This probably is not a priority for sphinxcontrib-jupyter, as Python 2 is being phased out, and this prevents using Python 3 goodies. However this would be helpful for the SageMath project: we are working hard on Python 3 support, but it will still take months (years?) to get there, and we would like to use sphinxcontrib-jupyter. So we would appreciate if you could accept them.

I ran the tests with Python 2 and just the super fix. With this branch, they pass up to a failure that I also get with Python 3:

[FAIL] _build/jupyter/equation_labels.ipynb and ipynb/equation_labels.ipynb are different:
[{'diff': [{'diff': [{'diff': [{'diff': [{'valuelist': u'-', 'key': 18, 'op': u'addrange'}, {'length': 1, 'key': 18, 'op': u'removerange'}], 'key': 11, 'op': u'patch'}, {'diff': [{'valuelist': u'-', 'key': 27, 'op': u'addrange'}, {'length': 1, 'key': 27, 'op': u'removerange'}], 'key': 24, 'op': u'patch'}, {'diff': [{'valuelist': u'-', 'key': 18, 'op': u'addrange'}, {'length': 1, 'key': 18, 'op': u'removerange'}], 'key': 55, 'op': u'patch'}, {'diff': [{'valuelist': u'-', 'key': 34, 'op': u'addrange'}, {'length': 1, 'key': 34, 'op': u'removerange'}], 'key': 82, 'op': u'patch'}], 'key': u'source', 'op': u'patch'}], 'key': 0, 'op': u'patch'}], 'key': u'cells', 'op': u'patch'}]

The unicode fixes are not exercised by the current tests; I just added them one by one until all our documentation compiled (note: compiling our doc also required some other upcoming patches by @ocayrol).

Thanks in advance!

mmcky commented 6 years ago

thanks @nthiery. I don't see this being an issue. I will take a closer look at this tomorrow morning and then I should be able to merge.

mmcky commented 6 years ago

and this prevents using Python 3 goodies

@nthiery do you happen to know of a good resource that tracks what can't be used in python3 to enable this to be python2 and python3 compatible? I will add python2 to travis to ensure compatibility for future PR's

nthiery commented 6 years ago

[1]@nthiery do you happen to know of a good resource that tracks what can't be used in python3 to enable this to be python2 and python3 compatible?

Nothing better than the main doc:

Browsing through it again, I realize it would be much better to use

    from __future__ import unicode_literals

than manually tweak strings one by one. Let me know if you would like me to update my PR, or if you will take care of it.

Presumably it would be safe thing to use some of the other future goodies like:

    from __future__ import division

I will add python2 to travis to ensure compatibility for future PR's

Thanks!

mmcky commented 6 years ago

Thanks @nthiery I think its a great idea to add imports from __future__. Thanks for the links - I had not come across the second link before - that is really handy. I haven't maintained cross version projects so this is a bit new to me. If you can review my changes and let me know if that is what you had in mind that would be great. Thanks.

nthiery commented 6 years ago

That's exactly what I had in mind; thanks!

Looking forward the happy time when we won't have to worry about Python 2 anymore :-)

mmcky commented 6 years ago

Thanks @nthiery. Indeed.