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

Restore support for py3.5 by removing f-string in setup.py #3662

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

amykyta3
Copy link
Contributor

Python3 runtime's setup.py introduces the use of an f-string which needlessly breaks compatibility with Python3.5 (#3650)

Remove the use of an f-string to restore ability to use runtime in python3.5

@amykyta3 amykyta3 changed the base branch from master to dev April 16, 2022 05:53
@amykyta3 amykyta3 changed the title Remove f-string in Python3 runtime setup.py Restore support for py3.5 by removing f-string in setup.py Apr 18, 2022
@amykyta3 amykyta3 force-pushed the fix-py3-setup branch 2 times, most recently from e7405cc to e6b3f68 Compare April 20, 2022 03:21
@amykyta3
Copy link
Contributor Author

@parrt: Fixed the DCO and merge conflict issues.
Confirmed that removing the f-string restores the ability to install the package using Python 3.5:

$ python3.5 -m pip install ./antlr4/runtime/Python3/
Processing ./antlr4/runtime/Python3
  Ignoring typing: markers 'python_version < "3.5"' don't match your environment
Installing collected packages: antlr4-python3-runtime
  Running setup.py install for antlr4-python3-runtime ... done
Successfully installed antlr4-python3-runtime-4.10.1

@parrt
Copy link
Member

parrt commented Apr 20, 2022

hi. This shouldn't really be a necessary change given that installing Python 3 code should be able to use the python three syntax right?

@amykyta3
Copy link
Contributor Author

That's not universally true for Python3.
Formatted string literals (aka f-strings) were introduced in Python v3.6. See https://docs.python.org/3.6/whatsnew/3.6.html#pep-498-formatted-string-literals
Older versions like Python 3.5 will not be able to interpret f-strings, as demonstrated in the example I show in #3650.

Given that the setup.py file is the only part of the Python3 runtime that uses this feature, seems sensible to revert it to the older style % formatter so that any stragglers still using Python 3.5 can use the v4.10 runtime.

In the future if you plan on doing a more wholesale refactoring that introduces Python 3.6+ features, recommend adding a minimum Python version constraint in the setup.py. This will ensure users will not blindly pull a version of the runtime from pypi that is not compatible (pip will automatically find the last supported version).
For example:

setup(
    ...
    python_requires=">=3.6.0"
)

When published, this version constraint gets annotated in the project's JSON index in pypi. Eg: https://pypi.org/pypi/antlr4-python3-runtime/json

@KvanTTT
Copy link
Member

KvanTTT commented Apr 21, 2022

In the future if you plan on doing a more wholesale refactoring that introduces Python 3.6+ features, recommend adding a minimum Python version constraint in the setup.py

In this case I'd also recommend dropping support of Python 2 (#3266)

@parrt
Copy link
Member

parrt commented Apr 22, 2022

Hi @amykyta3 sorry about that. It's hard to keep track of which language version has white and there are like nine targets for ANTLR to keep track of.

@parrt parrt added this to the 4.10.2 milestone Apr 22, 2022
@parrt parrt merged commit ae1f3f1 into antlr:dev Apr 22, 2022
@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

@parrt Is there a current planned date for the 4.10.2 release, or is there a workaround for Python version <=3.5? Without one, there's no clean way to install 4.10.x in affected Python versions today.

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

Oh actually I'm realizing the problem isn't even fixed for Python 2: https://github.com/antlr/antlr4/blob/master/runtime/Python2/setup.py#L13

@KvanTTT
Copy link
Member

KvanTTT commented Aug 12, 2022

Maybe now is the right time to deprecate Python 2 runtime since it doesn't look working now? #3266

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

If this is the only thing standing between 4.10, which is a major version with new features, and Python 2 support, then it makes more sense to fix this literally one-line bug so we have a working Python 2 version. Obviously it doesn't make sense to support Python 2 in perpetuity, but I think we can be more deliberate about removing support than that. Especially since here literally 99.9999999% of the work to get a Py2-compatible 4.10 version is done.

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

If @parrt doesn't want to rush out a 4.10.2 version I can try to do so on a fork. Are there any other known problems with 4.10.1 for Python 2?

@parrt
Copy link
Member

parrt commented Aug 12, 2022

hi. can you be more specific about what the install issue is? It works on my mac box for 2 and 3:

critter:~ $ pip install antlr4-python3-runtime
Collecting antlr4-python3-runtime
  Downloading antlr4-python3-runtime-4.10.tar.gz (116 kB)
     |████████████████████████████████| 116 kB 8.8 MB/s 
Building wheels for collected packages: antlr4-python3-runtime
  Building wheel for antlr4-python3-runtime (setup.py) ... done
  Created wheel for antlr4-python3-runtime: filename=antlr4_python3_runtime-4.10-py3-none-any.whl size=144174 sha256=1d1a03b68755e601d9864e2a007be6a3a818c9e70eaf5bfd38b4250b0c2198a4
  Stored in directory: /Users/parrt/Library/Caches/pip/wheels/b6/ad/fa/3b3f55bd8f984116397a4da5b33a4f9be7df18c2be0ba972e4
Successfully built antlr4-python3-runtime
Installing collected packages: antlr4-python3-runtime
Successfully installed antlr4-python3-runtime-4.10
critter:~ $ pip install antlr4-python2-runtime
Collecting antlr4-python2-runtime
  Downloading antlr4-python2-runtime-4.10.1.tar.gz (113 kB)
     |████████████████████████████████| 113 kB 8.7 MB/s 
Building wheels for collected packages: antlr4-python2-runtime
  Building wheel for antlr4-python2-runtime (setup.py) ... done
  Created wheel for antlr4-python2-runtime: filename=antlr4_python2_runtime-4.10.1-py3-none-any.whl size=140261 sha256=855302700705663199326c907d92126388ec36d4f2f2c7acec5fb8ad98307341
  Stored in directory: /Users/parrt/Library/Caches/pip/wheels/61/da/bd/fa250ef80d1e71b7d053fadd8706ef15b9f3c761dd88471b76
Successfully built antlr4-python2-runtime
Installing collected packages: antlr4-python2-runtime
Successfully installed antlr4-python2-runtime-4.10.1

Oh. I'm 3.9 python. Is that the issue? I need like 3.4?

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

You can't install this with a Python 2 version of pip, since it doesn't know how to read the F string. So right now the Python 2 runtime is not installable under any Python 2 version.

Collecting antlr4-python2-runtime==4.10
  Downloading https://files.pythonhosted.org/packages/f9/e6/fe9b7af4b30d48604359241d7fc5483c8d4dee1347ff9a0c29e00fc83230/antlr4-python2-runtime-4.10.tar.gz (113kB)
     |████████████████████████████████| 122kB 6.4MB/s
    ERROR: Command errored out with exit status 1:
     command: /usr/local/opt/python@2/bin/python2.7 -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/var/folders/wk/_wsd7yrj3ql9q637l6m4cp2w0000gn/T/pip-install-4iE6kG/antlr4-python2-runtime/setup.py'"'"'; __file__='"'"'/private/var/folders/wk/_wsd7yrj3ql9q637l6m4cp2w0000gn/T/pip-install-4iE6kG/antlr4-python2-runtime/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /private/var/folders/wk/_wsd7yrj3ql9q637l6m4cp2w0000gn/T/pip-install-4iE6kG/antlr4-python2-runtime/pip-egg-info
         cwd: /private/var/folders/wk/_wsd7yrj3ql9q637l6m4cp2w0000gn/T/pip-install-4iE6kG/antlr4-python2-runtime/
    Complete output (6 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/private/var/folders/wk/_wsd7yrj3ql9q637l6m4cp2w0000gn/T/pip-install-4iE6kG/antlr4-python2-runtime/setup.py", line 13
        description=f'ANTLR {v} runtime for Python 2.7.12'
                                                         ^
    SyntaxError: invalid syntax
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

@parrt
Copy link
Member

parrt commented Aug 12, 2022

Ah I see. Yeah, i don't even have python2.

Yeah, I'm hoping to get 4.10.2 soon but I'm in process with a bunch of stuff. :(

@parrt
Copy link
Member

parrt commented Aug 12, 2022

We should tweak the setup file though to support earlier python 3 too.

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

I'll send a PR and try to get a version out on a fork

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

We should tweak the setup file though to support earlier python 3 too.

I think this was already covered by the PR we're commenting on.

@parrt
Copy link
Member

parrt commented Aug 12, 2022

Oh right. read the diff wrong. So you just need it to run with python2, is that the issue? (Sorry 8 conversations going in my head simultaneously this morning)

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

There are separate setup.py files for Python 2 and 3. The 3 one was fixed, I'll fix the 2 one.

@thieman
Copy link
Contributor

thieman commented Aug 12, 2022

Seems to be working in Py2 with the setup.py fix, I've published a fork for now while we wait for official 4.10.2 release:

pip2 install antlr4-python2-runtime-thieman==4.10.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants