-
Notifications
You must be signed in to change notification settings - Fork 369
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
Update geoaxes.py pcolor to catch the ValueError #1476 #1479
Update geoaxes.py pcolor to catch the ValueError #1476 #1479
Conversation
pcolor crash SciTools#1476: catch the ValueError and remove the artist
So if I understand this correctly, currently we have cases where calls to I feel like the error is the proper behavior, otherwise, since users frequently do not read or pay attention to warnings, we're going to get bug reports about missing plots. I feel like the right solution is:
|
@dopplershift, yes we have draw time failures with pcolor, for cells that cannot be plotted by pcolor. As you suggest we have three options:
As to do it upfront, the quickest way to find the cells it cannot plot is to try to draw them as the best way to know which cells are not compatible with the source and destination projections using shapely is to go through the transformation with shapely, which is done at draw time. Below is a plot with cells that can be plotted with pcolor (lon starting at -175) and cells that cannot be plotted with colors (lon starting at -176). The crashes happens in shapely: The advantage of 2 is for pcolormesh plot. In that case only a couple of cells at the boundary are sent to pcolor for plotting to manage wrapping and they crash the whole plot with I feel that 2 is a progress as we go from a crash to a partial plot that would be totally acceptable for pcolormesh case. There may be a 2.5 way to deal with it. After removing the artist that fails, it could be possible to try to plot each cell in turn and only remove those that crashes at draw time. I am afraid it may be quite time consuming but it may be worth testing. |
Following the above comment, the following code removes only the cells that raises the value error
This is the code of an example that removes only the artist of the cells that crashes shapely.
This is the console output:
The left plot doesn't crash but the right one does. As this new code manages the exceptions, only the cell that crashes (the pink one) is not plotted while the others are properly plotted. This code cannot be transferred as is in the PR as there are many cases to manage, cmap, facecolors, edgecolors... so it would need some lines of code and some testing. It could be done in a further improvement. |
Analysis of the cause of the crash.Summary Images One of the polygons of the pink cell after projection to the plot projection. The cell is defined as a hole: The polygon after intersection (and inversion) with the projection boundary using shapely: Resulting data Those lineString contains only two points and cannot be converted to linearRing hence the crash. Further analysis Line 617 in 136a047
And further done the line, in shapely geometry multipolygon, it considers that if the object is not a polygon, then the first one is the outer shell and the others are the holes. So in our case, it considers that the first linestring is the outer shell and that's what crashes further done the line.
Possible fixes
In the mean time, I proposed to go through with the PR. |
Test of a fix following the above analysis:
SummaryIn some case the projected cell is transformed in a shapely geometryCollection that contains segments (lineString with 2 points) that crash in other shapely functions. Modification to crs.pyLine 617 in 136a047
ResultIt doesn't crash the plot anymore due to lineString in a geometryCollection. ImplicationsIt does modify crs in a function that may be used elsewhere. Code to reproduce the plot
|
Analysis of why crs.py _rings_to_multi_polygon issue of big cellsSummaryAfter fixing the valueError @dopplershift - the valueError exception Example on why the holes need to be joined before inversionIn the example below a polygon abcda is defined by three holes and a projection boundary.
The holes defining abcd are: The result expected is the polygon abcda. The image below shows:
The expected result is the right plot, with the union of the holes before the inversion. Code to reproduce the example
Fixing crs.py to union the holes before inversionThe code Line 590 in 136a047
should be modified according to the following, on top of the modification to handle the valueError exception:
I suggest to drop the current geoaxes PR for a crs PR. ImplicationsThis fixes a geometry union/difference error. |
It looks like you've figured out what to do here. Can you open a new PR with your proposed change for the hole calculations? (Please also give your commits and PRs more descriptive messages.) |
@QuLogic. I will open a new PR to clean all this up. |
pcolor crash #1476: catch the ValueError and remove the artist
Rationale
In some cases, pcolor raises a ValueError and crashes when plotting data from one projection to an other see #1476
It comes from the fact that it cannot transform one of the paths to the axis projection.
Instead of crashing the plot, this modification catches the error in pcolor and removes the faulty artist from the axis to avoid further crashes in draw().
Only once this PR is accepted, it will be possible to add a pcolor crash test in the test suite.
A better result would be obtained by plotting each path (grid cell) in turn and only removing the cells that crashes pcolor. That would be very time consuming see #1476 for an example.
This is a modification already proposed in #1467 from which it can be removed to simplify it, this is a first baby step.
Implications
The only implication to my knowledge is that it doesn't crash.