robshakir / pyangbind

A plugin for pyang that creates Python bindings for a YANG model.
Other
204 stars 121 forks source link

Not compatible with Python 3.6 / 3.7 due to enum34 requirement? #232

Closed kqchen34 closed 11 months ago

kqchen34 commented 6 years ago

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'

zvezdan commented 6 years ago

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.

tarkatronic commented 6 years ago

@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.

zvezdan commented 6 years ago

@tarkatronic

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.

tarkatronic commented 6 years ago

@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.

kqchen34 commented 6 years ago

@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.

kqchen34 commented 6 years ago

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."

ApheleiaS commented 6 years ago

Any updates on this issue? @tarkatronic

kqchen34 commented 6 years ago

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.

zvezdan commented 6 years ago

@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

kqchen34 commented 6 years ago

@zvezdan I see, makes sense, thanks! @tarkatronic When do you think we can have a new release to resolve this issue? Thanks!

tarkatronic commented 6 years ago

@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.

robshakir commented 6 years ago

I’ll look this afternoon.

r. On Mon, Dec 3, 2018 at 13:35 Joey Wilhelm notifications@github.com wrote:

@kqchen34 https://github.com/kqchen34 I have added a PR, #233 https://github.com/robshakir/pyangbind/pull/233, for this. I need to wait for a review from @robshakir https://github.com/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.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/robshakir/pyangbind/issues/232#issuecomment-443879091, or mute the thread https://github.com/notifications/unsubscribe-auth/AEnN1aWqxxC0x-kpx2wbiq4j7XOnQ2Vpks5u1ZkhgaJpZM4YCrDu .

kqchen34 commented 6 years ago

@tarkatronic @robshakir I tested use the branch in the PR request, it works! Thanks!

kqchen34 commented 5 years ago

@tarkatronic @robshakir Any update on this? When do you expect we can release this to pypi? Thanks.

ApheleiaS commented 5 years ago

@tarkatronic @robshakir Any update on releasing this fix?

jvleminc commented 2 years ago

Any chance this can be fixed?

JoseIgnacioTamayo commented 11 months ago

Latest versions of pyangbind are able to work with enum34 and Python >= 3.7

$ pip show enum34 Name: enum34 Version: 1.1.10 Summary: Python 3.4 Enum backported to 3.3, 3.2, 3.1, 2.7, 2.6, 2.5, and 2.4 Home-page: https://bitbucket.org/stoneleaf/enum34 Author: Ethan Furman Author-email: ethan@stoneleaf.us License: BSD License Required-by: pyangbind