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

Can requirements be bumped ? (numpy, scipy, pandas) #7322

Closed
alexandreCameron opened this issue Oct 28, 2021 · 11 comments · Fixed by #7470 or #7672
Closed

Can requirements be bumped ? (numpy, scipy, pandas) #7322

alexandreCameron opened this issue Oct 28, 2021 · 11 comments · Fixed by #7470 or #7672
Assignees
Milestone

Comments

@alexandreCameron
Copy link

Context

Currently, the package limits the version of third party packages such as: numpy, scipy, pandas

https://github.com/gem/oq-engine/blob/master/setup.py

install_requires = [
    'setuptools',
    'h5py >=2.10',
    'numpy >=1.18, <1.20',
    'scipy >=1.3, <1.7',
    'pandas >=0.25, <1.3',
    'pyzmq <20.0',
    'psutil >=2.0, <5.7',
    'shapely >=1.7, <1.8',
    'docutils >=0.11',
    'decorator >=4.3',
    'django >=3.2',
    'matplotlib',
    'requests >=2.20, <3.0',
    'pyshp ==1.2.3',
    'toml',
    'pyproj >=1.9',
]

Version 1.18 of numpy more than one year old.

These requirements limit the usage of the open-quake package in project other dependencies.

Question

Is it possible to run the package with more recent versions of the requirements ?

What is danger if the open-quake is run with newer version of third party packages ?

@micheles
Copy link
Contributor

We cannot upgrade numpy unless we drop Python 3.6. We have decided to drop Python 3.6 before the end of the year, so you have just to wait a little.

@alexandreCameron alexandreCameron changed the title Can requirements package bumped (numpy, scipy, pandas) Can requirements package be bumped ? (numpy, scipy, pandas) Oct 28, 2021
@alexandreCameron alexandreCameron changed the title Can requirements package be bumped ? (numpy, scipy, pandas) Can requirements be bumped ? (numpy, scipy, pandas) Oct 28, 2021
@alexandreCameron
Copy link
Author

@micheles , does your answer imply that I can drop all the version upper-limitation on requirements if I run open-quake with python version >=3.7 ?

@micheles
Copy link
Contributor

Yes, if you manually change the setup.py and remove the upper limits.

@micheles micheles added this to the Engine 3.13.0 milestone Oct 28, 2021
@micheles
Copy link
Contributor

micheles commented Dec 13, 2021

We should remove all upper bounds in the future (I have already removed some; they are a bad idea, but I am innocent in this case ;-)) See https://iscinumpy.dev/post/bound-version-constraints/

@alexandreCameron
Copy link
Author

@micheles , thanks a lot for removing the limitation and happy New Year🥳

I'm going to be a little bit annoying but you still kept a pyshp ==1.2.3 (released on the 22nd June 2015)
https://pypi.org/project/pyshp/

@micheles
Copy link
Contributor

micheles commented Jan 5, 2022

This was the only version that worked. If you can tell me which is another versions that works (i.e. all of our tests are green) then I will be happy to upgrade.

@alexandreCameron
Copy link
Author

@micheles , I've had a look at the code base, the issues seems on pyshp 's side :

GeospatialPython/pyshp#217 (comment)

I won't have time to contribute on the code base of the oq-engine and the pyshp projects.

I don't know all the implication of pyshp in the oq-engine project but pyshp ==1.2.3 was published without any CI/CD

https://github.com/GeospatialPython/pyshp/tree/1.2.3

I forgot to mention but since the opening of the issue Descartes Underwriting as become a GEM partner

https://www.descartesunderwriting.com/descartes-underwriting-enters-joint-partnership-with-gem/

@micheles
Copy link
Contributor

micheles commented Jan 7, 2022

Personally, I would be in favor of removing any support for shapefiles from the engine, so we could drop the dependency from pyshp. But it depends on our scientists.

@karimbahgat
Copy link

Hi everyone, this is the maintainer of pyshp. The issue with upgrading the pyshp version is not related to a lack of python 3 support, pyshp has continuous testing for 2.7 and 3.5-9. I'm not sure which specific errors are being encountered, but i'd be willing to bet it's api related, since the bump from v1 to v2 included some backwards incompatible changes. However, the changes are relatively simple and shouldn't be very hard to update in the openquake codebase. The v2 api changes are listed here, and the most recent api is available in the readme.

From a very brief look at the openquake code, i found at least one part that needs to be updated to be compatinle with v2. In 'shapefileparser.ShapefileParser.write', the old way of creating a writer 'shapefile.Writer(shapefile.POLYGON)' needs to be updated to the new way 'shapefile.Writer('save_path.shp', shapefile.POLYGON)'. In the new version, the writer writes each record directly to disk, instead of storing everything in memory and only saving at the end. This means the '.save()' parts have to be removed as well. I think these changes may be all that's needed to upgrade, unless there's other code dealing with shapefiles i'm not aware of.

Hopefully that helps, let me know if there's any questions.

@micheles
Copy link
Contributor

micheles commented Jan 27, 2022

Thank you very much for the helpful insight, @karimbahgat . Currently GEM is not using the Shapefile converter so we are inclined to remove completely the dependency from pyshp, also because all the new typologies of sources (new since 6 years ago or so) are not convertible in Shapefiles. @alexandreCameron do you use the Shapefile converter or not? If you don't use it directly and your only concern is about the dependency,dropping it would be best for you too.

@micheles micheles reopened this Jan 27, 2022
@alexandreCameron
Copy link
Author

@micheles , we are currently not using this feature, dropping the feature and the dependency would work out for us

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