Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Python 2 compat: avoid 'nonlocal' statement #274

Closed
wants to merge 1 commit into from
Closed

Fix Python 2 compat: avoid 'nonlocal' statement #274

wants to merge 1 commit into from

Conversation

pilou-
Copy link

@pilou- pilou- commented Jan 5, 2019

Python 2 compatibility is broken since 0.4.0 / 46b3664.

avoid nonlocal statement: wrap open_cite_tag inside a mutable object.

$ python --version
Python 2.7.15+
$ python setup.py build_sphinx
running build_sphinx
Running Sphinx v1.8.3

Exception occurred:
  File "/tmp/nbsphinx.venv/local/lib/python2.7/site-packages/Sphinx-1.8.3-py2.7.egg/sphinx/registry.py", line 472, in load_extension
    mod = __import__(extname, None, None, ['setup'])
  File "/tmp/nbsphinx/src/nbsphinx.py", line 1088
    nonlocal open_cite_tag
                         ^
SyntaxError: invalid syntax

wrap open_cite_tag inside a mutable object
@mgeier
Copy link
Member

mgeier commented Jan 5, 2019

Thanks for this PR!

Do you actually need Python 2 support?

I was thinking about removing Python 2 support, see also #268.

@pilou-
Copy link
Author

pilou- commented Jan 5, 2019

Lago requires nbsphinx and is still not compatible with Python 3. While Lago could support Python 3 soon, I guess Lago won't drop Python 2 support.

mgeier added a commit that referenced this pull request Jan 7, 2019
* remove use of "nonlocal" keyword, see #274
* work around one use of `list.clear()`
* switch back to "universal" wheels, which reverts #268
@mgeier
Copy link
Member

mgeier commented Jan 7, 2019

OK I've created #275.

I'm not using your PR because I don't like the "wrap it in a list" work-around for nonlocal. I prefer the "use a function attribute" work-around.

I can keep Python 2 compatibility for some more time, but at some point it will have to go away. In about a year the official support for Python 2 will end anyway ...

@mgeier
Copy link
Member

mgeier commented Jan 10, 2019

I've merged #275.

@mgeier mgeier closed this Jan 10, 2019
@pilou-
Copy link
Author

pilou- commented Jan 10, 2019

@mgeier thanks !

@jeffrey-hokanson
Copy link

Thanks for doing this. I've manually installed updated version, but the changes introduced in this merge aren't appearing in the latest pip version and, consequently, I'm having trouble using nbsphinx on readthedocs. Would a version bump fix this?

@mgeier
Copy link
Member

mgeier commented Jan 13, 2019

@jeffrey-hokanson I haven't made a new release yet, but I will make one within the next few days.

In the meantime, if you are in a hurry, you can use the master branch of nbsphinx on readthedocs as described in the docs https://nbsphinx.readthedocs.io/en/0.4.1/usage.html#Automatic-Creation-of-HTML-and-PDF-output-on-readthedocs.org.

@mgeier
Copy link
Member

mgeier commented Jan 15, 2019

I've just uploaded the new release 0.4.2 to PyPI.

mgeier added a commit that referenced this pull request Feb 3, 2019
* remove use of "nonlocal" keyword, see #274
* work around one use of `list.clear()`
* switch back to "universal" wheels, which reverts #268
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.

3 participants