search

matplotlib / basemap

Plot on map projections (with coastlines and political boundaries) using matplotlib
MIT License
779 stars 392 forks source link

Update geos version number #498

Closed joernu76 closed 4 years ago

joernu76 commented 4 years ago

Fix issue #496

joernu76 commented 4 years ago

I removed 2.6, but kept 2.7 for now. I also added python 3.7+3.8 but removed 3.4 and 3.5.

Now the tests would likely pass when issue #495 would be adressed (all failing test cases show the "dedent" failure.

When accepting this and the #495 fix, travis will likely suceed, otherwise I can have another look.

WeatherGod commented 4 years ago

Going to power-cycle the PR to see how it holds up now that I accepted #496.

WeatherGod commented 4 years ago

Looks like something isn't right with libgeos v3.6.1. We are getting a segfault during the tests.

joernu76 commented 4 years ago

Why did the tests not segfault in the pipeline for my merge request? https://travis-ci.org/github/matplotlib/basemap/builds/714811873

joernu76 commented 4 years ago

Ah, because the dedent was fixed. Will see what I can do to fix this.

joernu76 commented 4 years ago

The tests pass now. There was a "closed" issue (#484) pointing out this exact error and a solution. I modified the solution slightly. GEOS doesn't accept infinites any more and I added a workaround for this that has likely no adverse implications. I do not fully understand the code at that point in time though. The fix cannot be worse than the crash, though...

WeatherGod commented 4 years ago

And you see why I want to dump this codebase like the hot mess that it is?

Yeah, this fix looks reasonable, but it makes me worry that there are similar issues elsewhere in the codebase. The unit tests aren't nearly extensive enough. I am going to put it though some more paces by having it generate the documentation on my machine as well.

WeatherGod commented 4 years ago

The examples are reminding me why I have been wanting to drop this library for so long, as some of them are broken for various reasons, but most are fine, and nothing seems broken by this fix, so it should be a net improvement. Merging.