Switch to storing CRS WKT in AreaDefinitions instead of the CRS object #264
Conversation
|
Oh, also, I've added a workaround for this issue to my own processing to see if it helps like I think it should. If the processing still fails then that could mean this isn't the solution we need. |
|
On slack @pnuu asked about what version of WKT is used internally. Pyproj defaults to WKT2 but I asked pyproj4/pyproj#604 to get some more details. I was concerned that there may be a problem where pyproj wants to create WKT2 but the installed PROJ version doesn't support it. This is not an issue since the CRS object is only available in pyproj 2.0+ and pyproj 2.0+ requires PROJ 6 which is providing the WKT2. |
Codecov Report
@@ Coverage Diff @@
## master #264 +/- ##
==========================================
- Coverage 91.94% 91.92% -0.03%
==========================================
Files 42 42
Lines 8098 8171 +73
==========================================
+ Hits 7446 7511 +65
- Misses 652 660 +8
Continue to review full report at Codecov.
|
|
Maybe it would be good to also test the usage of WKT as projection definition in |
Co-Authored-By: Panu Lahtinen <[email protected]>
|
@pnuu I updated this with a test for different inputs to AreaDefinition (proj dict, proj string, WKT2, WKT1). The only surprise was that WKT1 has some different naming for unknown parameters so a CRS from WKT2 does not equal a CRS created from WKT1. I've commented out that equality check. |
| 'lon_0': '8.00', | ||
| 'proj': 'stere' | ||
| } | ||
| crs = CRS(CRS.from_dict(proj_dict).to_wkt()) |
There was a problem hiding this comment.
Just curious: What is the benefit on the extra CRS(crs.to_wkt()) layer here?
There was a problem hiding this comment.
It is needed otherwise the CRS objects are not strictly equal later on. The conversion is done inside the AreaDefinition so when I do area_def.crs == crs they are the same. If I remember correctly if I don't do it then there is some information gained or lost in the conversion to WKT that makes the CRS objects not equal.
There was a problem hiding this comment.
Just checked and the WKT of the two CRS objects is the same, but something in the CRS based on the PROJ dict is not equivalent.
Edit: My guess is that when it is converted to WKT it gets a/b converted to a/rf and PROJ is not considering these equivalent or isn't willing to make an assumption on floating point equality.
There was a problem hiding this comment.
Oh boy! Thanks for sorting out all these nasty details!
| geos_area.proj_dict['a'] = 1000.0 | ||
| geos_area.proj_dict['b'] = 1000.0 | ||
| geos_area.proj_dict['h'] = np.sqrt(2) * 1000.0 - 1000.0 |
This PR is an effort to resolve the issues discussed in:
pyproj4/pyproj#589
pytroll/satpy#1114
The summary of these issues is that the pyproj
CRSobject is not thread-safe. If you access one from multiple threads you can get strange corrupt strings and bytes. I've found one case in all of satpy, pyresample, and trollimage (I think) where we explicitly pass a CRS or Proj object to a dask function (map_blocks, blockwise, delayed) and that's what this PR fixes (call toinvprojwith aCRSobject). There is an additional usage in the EWA resampling in Satpy where an AreaDefinition is passed to a delayed function. I would like to be able to confidently pass AreaDefinitions between threads so that issue is also addressed here.The main switch is that AreaDefinition objects will now store
crs_wkt(the WellKnownText string of a CRS object) which can be used to reconstruct theCRSobject. Since it is a string it is perfectly thread-safe. This has the unintended consequence that if you pass a PROJ dict to an AreaDefinition and then doarea_def.proj_dictyou will get the result ofdict -> CRS -> WKT -> CRS -> dict. Due to the simplifications that pyproj and PROJ do this can result in completely different PROJ dicts being produced which can be confusing and can require rewriting a lot of tests which I also tried to do here. Some examples of the simplications:ellps.However, I'm also finding a few bugs in pyproj/PROJ which I've brought up here: pyproj4/pyproj#592. I consider this PR a WIP until those can get resolved and the tests in pyresample fixed.
git diff origin/master **/*py | flake8 --diff