Not compatible with Python 3.6 / 3.7 due to enum34 requirement?
kqchen34 opened this issue · 17 comments
If enum34 is a requirement of pyangbind, then pyangbind would not be able to support Python 3.6 / 3.7, since the lib enum34 is not compatible with Python 3.6 and later versions.
E.g. when I try to run an application in python 3.7 with pyangbind imported, I got the following error:
Traceback (most recent call last):
File "bin/generate_yang_modules", line 49, in
import app_scripts.generate_yang_modules
File "/bbnet/home/kchen34/git/bnop_driver/app_scripts/init.py", line 2, in
import("pkg_resources").declare_namespace(name)
File "/bb/bin/package/p/python3.7-venv-bnop/1.0.1+b20181029T17473333/libexec/python3.7-venv-bnop/lib/python3.7/site-packages/pkg_resources/init.py", line 24, in
import re
File "/bb/sys/package/p/python3.7-venv-bnop/1.0.1+b20181029T17473333/libexec/python3.7-venv-bnop/lib/python3.7/re.py", line 143, in
class RegexFlag(enum.IntFlag):
AttributeError: module 'enum' has no attribute 'IntFlag'
This dependency needs to be declared conditionally for python 2.7 only using environment markers.
There are several options here:
enum34 ; python_version<'3'
This is the most straightforward way. You can also use =='2.7' or <3.4, which based on the versions of Python already supported by pyangbind are equivalent to the above marker.
Notice that this kind of setting requires a recent version of setuptools (>=36.2) because markers were not allowed in install_requires before that. Since setup.py has code in it that processes requirements.txt and puts them in install_requires, the new markers syntax in them would demand the use of recent setuptools.
If pyangbind developers want to make sure that it can be built with older version of setuptools, then the approach must be different. The code that reads requirements.txt would have to exclude any lines that have a semicolon in them (markers present) and put those entries in extras_require instead of install_requires. The format in that case would be:
extras_require={
":python_version<'3'": ['enum34'],
}
This would work with setuptools>=18.0.
This should be easy to fix and ensure that pyangbind produces correct Python package metadata.
@kqchen34 Could you possibly share code and/or models to reliably reproduce this? We aren't seeing this currently in our test suite, so I would like to add a test in order to expose it and verify it's fixed.
It's easy to verify these kinds of things by looking in the package metadata.
Download pyangbind-0.8.1.tar.gz from pypi.org. Open the tar archive. Take a look at pyangbind.egg-info/requires.txt. It looks like this:
pyang
bitarray
lxml
regex
six
enum34
and it should look like this:
pyang
bitarray
lxml
regex
six
[:python_version<'3']
enum34
As mentioned above, replace <'3' with =='2.7' or <'3.4' if you prefer.
You can see this immediately after running python setup.py sdist during the publication of the package.
If you try building a wheel, then the METADATA file in the dist-info directory should have:
Requires-Dist: enum34 ; python_version<'3'
Simply, the metadata of pyangbind needs to be fixed.
Forcing old, probably already abandoned (even if it isn't, it will be in the future) backport package on users of Python 3.7 (or anything >=3.4) is not only unnecessary, but can create issues by overriding working standard library module that can add new APIs compared to the old backport package.
A build systems may enforce exclusion of backport packages for versions of Python that have them in standard library. This usually means the backport packages are allowed only with Python 2.7, now that support for 3.4 has been dropped.
@zvezdan I understand your proposal, but it is likely unrelated to the issue which @kqchen34 is having. The test suite currently runs against Python 2.7, 3.4, 3.5, 3.6, and 3.7.
While having that package changed to a conditional import is a good idea, and should be done, it is not currently triggering an error in the test suite. I would like to get a test added to expose the error reported here, so that we can verify that this is the only thing which needs to be changed.
@zvezdan thanks for the suggestion on the solution, the solution can only be done at pyangbind library level, right? Is there a way we can fix at the application level for the application that is using pyangbind library? In case we need the fix to move our application to production before pyangbind library has a fix?
@tarkatronic The replication on the error is very simple, as long as your code import the library pyangbind, and use buildout, the eggs will include lib enum34 since it is part of requirements by pyangbind. And then start your program in python 3.7, it will give the error mentioned in my previous post:
AttributeError: module 'enum' has no attribute 'IntFlag'
=====The workaround I am taking now, is to manually delete the lib enum3.4 (enum34-1.1.6-py3.7.egg) from my eggs directory after I ran buildout.====
So the proposal posted by zvezdan should fix the issue I am having. I.e. in pyangbind 's metadata, do not require enum34 is the python version is < 3.4.
Refer to:
https://bitbucket.org/stoneleaf/enum34/issues/19/enum34-isnt-compatible-with-python-36
Can we have a fix in this lib soon please?
Thanks.
Sorry, correction to my previous comment:
"do not require enum34 is the python version is < 3.4."
should read
"only require enum34 if the python version is < 3.4."
Any updates on this issue? @tarkatronic
Can we have a quick fix/release for this issue please?
The change should be very simple:
(1) remove enum34 from requirements.txt
(2) add the following lines in setup.py:
if sys.version_info < (3,):
requirements.append("enum34")
Similar to the following examle:
https://github.com/ktbyers/netmiko/blob/develop/setup.py#L12
Thanks.
@kqchen34 Using conditional statements in setup.py may produce incorrect data again.
Instead, a proper metadata settings should be used.
Similar to this: https://gitlab.com/pycqa/flake8/merge_requests/191/diffs
@zvezdan I see, makes sense, thanks!
@tarkatronic When do you think we can have a new release to resolve this issue? Thanks!
@kqchen34 I have added a PR, #233, for this. I need to wait for a review from @robshakir before it can be merged and released.
If you are able to test with that branch, it would be a great help. I am still very concerned that the issue you are seeing is not exposed by any of our tests, so I can not confirm 100% that this change fixes your issue.
@tarkatronic @robshakir I tested use the branch in the PR request, it works! Thanks!
@tarkatronic @robshakir Any update on this? When do you expect we can release this to pypi? Thanks.
@tarkatronic @robshakir Any update on releasing this fix?
Any chance this can be fixed?