Skip to content

Update geos version number #498

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

Merged
merged 2 commits into from
Aug 4, 2020
Merged

Update geos version number #498

merged 2 commits into from
Aug 4, 2020

Conversation

joernu76
Copy link
Contributor

@joernu76 joernu76 commented Aug 3, 2020

Fix issue #496

@joernu76
Copy link
Contributor Author

joernu76 commented Aug 3, 2020

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
Copy link
Member

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

@WeatherGod WeatherGod closed this Aug 4, 2020
@WeatherGod WeatherGod reopened this Aug 4, 2020
@WeatherGod
Copy link
Member

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

@joernu76
Copy link
Contributor Author

joernu76 commented Aug 4, 2020

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

@joernu76
Copy link
Contributor Author

joernu76 commented Aug 4, 2020

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

Also deprecate old python versions and added current ones.

Fix issue #496
@joernu76
Copy link
Contributor Author

joernu76 commented Aug 4, 2020

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
Copy link
Member

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
Copy link
Member

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.

@WeatherGod WeatherGod merged commit 3076ec9 into matplotlib:master Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants