Pin shapely to >=2.0 #2075
Pin shapely to >=2.0 #2075
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2075 +/- ##
=======================================
Coverage 93.08% 93.08%
=======================================
Files 237 237
Lines 12793 12793
=======================================
Hits 11909 11909
Misses 884 884 |
|
Why do we need versions larger than 2? |
|
It's not a massively important pin, but 2.0+ has introduced a lot of changes, it's not that <2.0.0 would break stuff AFAIK though. @zklaus suggested it, and in Klaus I trust when it comes to these things 馃榿 |
|
I just ran some tests by creating an environment with shapely <2 (I got 1.8.5) and all our unit tests pass, so I would really like to have some better explanation of why we need this before we consider merging this. |
|
The feature freeze for the v2.9 release was yesterday, so I'm removing this from the milestone. |
|
no biggie, man - I think we should be perfectly safe without pinning it for now (remember you made me plop that pytest-json dep in for when an older esmpy gets installed due to an older shapely); so we can always pin if needed; let's just keep an eye on the package build for ESMValTool, there may be an issue, if at all |
|
Shapely 2 comes with a partly new API that facilitates much better performance due to consistent vectorization. This API is already available in version 1.8, which is a transitional version, i.e. API that is being removed in version 2 is merely deprecated in 1.8 and throws deprecation warnings. Imho, the current situation is slightly dangerous (only slightly because, after all, shapely is not used a lot in our code base). This is because people will likely get version 1.8 and might still use the old API, ignoring the deprecation warnings. When the new version 2 comes in those parts will break. If we pin to >2 now, we will see the breakage and be able to fix it; all further use of shapely will be forced to use only the new API. |
|
@zklaus thanks a lot, man! I wasn't aware of the API evolution in the 2+ versions, but I had a hunch we should go for the modern ones, hence this PR - @bouweandela I'd say we should resurrect this 馃拃 |
There was a problem hiding this comment.
Thanks for explaining! Could you please clean up @valeriupredoi?
Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
|
just did, monsieur le @bouweandela 馃嵑 |
|
Cheers bud 馃嵑 |
|
Ha! Merged 2min before getting off the bus, very shapely of me 馃榿 |
Description
We discussed starting from @zklaus comment and agreed that we should let shapely be modern again; we'd have to fix recipe_shapeselst.yml, as Klaus points out, but we better fix a recipe and have modern dependencies, than the other way round.
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 馃洜 Technical or 馃И Scientific review.
To help with the number pull requests: