GLIMS-RGI / rgitools

Processing files from the Randolph Glacier Inventory
http://rgitools.readthedocs.io
BSD 3-Clause "New" or "Revised" License
14 stars 9 forks source link

pip install fails with setuptools error #65

Open iamdonovan opened 1 year ago

iamdonovan commented 1 year ago

rgitools fails to install using pip in environments with newer versions of setuptools, due to a conflict with PEP440:

image

I was able to fix this by changing changing line 86 from this:

FULLVERSION = rev.lstrip('v').replace(VERSION + '-', VERSION + '+')

to this:

FULLVERSION = rev.lstrip('v').replace(VERSION + '-', VERSION + '+').replace('-', '+', 1)

This fixes the version number error, and rgitools installs as expected:

image

jhkennedy commented 1 year ago

It looks like the setup.py has a bunch of logic to build version string from the git repo -- I'd suggest stripping that out entirely and instead using: https://github.com/pypa/setuptools_scm

setuptools_scm should provide all the features you'd need, and will comply with PEP440 as well. And, after an admittedly quick glance, that would let you move entirely to setup.cfg or pyproject.toml in line with setuptools best practices.

fmaussion commented 1 year ago

Thanks all! Yeah this package is old - I'm not sure I'll continue to use it or simply move the useful scripts to https://github.com/GLIMS-RGI/rgi7_scripts

What did you need from it?

iamdonovan commented 1 year ago

Mostly the clustering tools: https://rgitools.readthedocs.io/en/latest/tools.html#merging-glacier-clusters

fmaussion commented 1 year ago

@iamdonovan RGItools was an over-engineered attempt to generalize workflows, I think you would be much faster in grabbing the code you need here: https://github.com/GLIMS-RGI/rgitools/blob/master/rgitools/funcs.py#L194-L375

One thing I am unhappy about in the current version of the code is that we have a hardcoded buffer in which we search for intersects: https://github.com/GLIMS-RGI/rgitools/blob/master/rgitools/funcs.py#L242 . For RGI7 I wondered if we could work in projection space (intead of LatLon space) and use a threshold in meter instead...