pycsw icon indicating copy to clipboard operation
pycsw copied to clipboard

pkg_resources force compliance with tight pinning

Open ocefpaf opened this issue 7 years ago • 3 comments

pycsw is using __version__ = pkg_resources.require("pycsw")[0].version to get the version, but calling pkg_resources.require will check for the dependencies versions and will raise an error when the tight pinning suggested in the requirements.txt is not followed. For example:

import: 'pycsw'
Traceback (most recent call last):
  File "/feedstock_root/build_artefacts/pycsw_1521545326347/test_tmp/run_test.py", line 2, in <module>
    import pycsw
  File "/feedstock_root/build_artefacts/pycsw_1521545326347/_t_env/lib/python3.6/site-packages/pycsw/__init__.py", line 35, in <module>
    __version__ = pkg_resources.require("pycsw")[0].version
  File "/feedstock_root/build_artefacts/pycsw_1521545326347/_t_env/lib/python3.6/site-packages/pkg_resources/__init__.py", line 892, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/feedstock_root/build_artefacts/pycsw_1521545326347/_t_env/lib/python3.6/site-packages/pkg_resources/__init__.py", line 783, in resolve
    raise VersionConflict(dist, req).with_context(dependent_req)
pkg_resources.ContextualVersionConflict: (lxml 4.2.0 (/feedstock_root/build_artefacts/pycsw_1521545326347/_t_env/lib/python3.6/site-packages), Requirement.parse('lxml==3.6.2'), {'pycsw'})

I personally don't like the use of pkg_resource for getting the version, there are better patterns to extract the version from a text file (or even better, use versioneer). However, the main issue is the side effect due to the tight pinning:

geolinks==0.2.0
lxml==3.6.2
OWSLib==0.16.0
pyproj==1.9.5.1
Shapely==1.5.17
six==1.10.0
xmltodict==0.10.2

For example, requiring the old shapely 1.5.17 leads to many downgrades when installing pycsw with other GIS packages that also require shapely. I bet that many, if not all, of those pinnings could be relaxed, or at least >= some version.

ocefpaf avatar Mar 20 '18 11:03 ocefpaf

@ocefpaf pkg_resources was introduced in as part of 4eb9a3320509d791d4619199c832d4481c3813f7. @ricardogsilva comments? Could we implement without pkg_resources? Proposed patch:

diff --git a/pycsw/__init__.py b/pycsw/__init__.py
index b017be0..853e3d0 100644
--- a/pycsw/__init__.py
+++ b/pycsw/__init__.py
@@ -30,6 +30,4 @@
 #
 # =================================================================
 
-import pkg_resources
-
-__version__ = pkg_resources.require("pycsw")[0].version
+__version__ = '2.3-dev'
diff --git a/setup.py b/setup.py
index cdc8655..68d16da 100644
--- a/setup.py
+++ b/setup.py
@@ -32,6 +32,7 @@
 
 import io
 import os
+import re
 
 from setuptools import find_packages, setup
 
@@ -43,6 +44,16 @@ def read(filename, encoding="utf-8"):
     return contents
 
 
+def get_package_version():
+    """get version from top-level package init"""
+    version_file = read('pycsw/__init__.py')
+    version_match = re.search(r"^__version__ = ['\"]([^'\"]*)['\"]",
+                              version_file, re.M)
+    if version_match:
+        return version_match.group(1)
+    raise RuntimeError("Unable to find version string.")
+
+
 # ensure a fresh MANIFEST file is generated
 if (os.path.exists('MANIFEST')):
     os.unlink('MANIFEST')

tomkralidis avatar May 30 '19 03:05 tomkralidis

guys I'm not trying to be picky, but perhaps you are pointing your guns at the wrong target with this one?

IMO the real issue here is that we are pinning versions in setup.py, which is an anti-pattern[1] - I think we should not do tight pinning in setup.py, even if it involves some minor duplication by having to maintain the package names in both requirements.txt and setup.py.

Regarding your proposed fixes:

@ocefpaf - maybe this does boils down to an opinion, but it seems to me that switching to versioneer will bring no advantage here. pkg_resources is part of setuptools which comes by default with recent python versions, and it actually does its job quite well, provided that we keep up with convention and don't do strict pinning in setup.py - which unfortunately we are doing, and hence your current troubles. I am aware that there are a myriad other ways to determine the version and other projects do it differently, but I've yet to see a more compelling approach than the pkg_resources one. Perhaps you have some other alternatives to point out that may be better?

@tomkralidis - your fix works OK, but in my opinion it is more of a hack, at least when compared with the current implementation

So, in my opinion, we'd rather fix the tight pinning of versions in setup.py instead of changing the way the version is read.

[1] - https://packaging.python.org/discussions/install-requires-vs-requirements/

ricardogsilva avatar May 30 '19 08:05 ricardogsilva

cc @kalxas

Thanks @ricardogsilva. The tight pinning in requirements.txt has since been lifted, so I'm wondering if this is an issue now (at least for master).

Note we are also discussing this in #589. Sounds like we should continue this issue there?

tomkralidis avatar May 30 '19 12:05 tomkralidis