-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
Fix handling wkt strings and add pyproj.CRS support #1139
Conversation
Thank you for finding the cause of the confusing behavior. Your changes stops the error, but I still find the behavior not to be ideal, e.g., import cartopy.crs as ccrs
import rioxarray
import xarray as xr
from hvplot.util import process_crs
crs = ccrs.RotatedPole(pole_longitude=177.5, pole_latitude=37.5)
ds = xr.Dataset(attrs={"crs": crs})
a1 = process_crs(ds.rio.crs.to_proj4())
a2 = process_crs(ds.attrs["crs"])
type(a1), type(a2) Outputs: After looking into it more, it seems these lines are the problem: Lines 130 to 131 in c049c7b
Looking at this: |
Good catch! I believe
Also, I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me.
One small thing is we have identical functions in GeoViews. Can you update them and test that we don't break anything in that CI, too, with these changes?
I think it is out-of-scope for this PR, but we are getting some warnings related to these tests. (Already there before this PR)
hvplot/tests/testgeo.py::TestProjections::test_plot_with_crs_as_nonexistent_attr_str
hvplot/tests/testutil.py::TestGeoUtil::test_proj_to_cartopy
hvplot/tests/testutil.py::test_check_crs
hvplot/tests/testutil.py::test_check_crs
hvplot/tests/testutil.py::test_process_crs[+init=epsg:26911]
hvplot/tests/testutil.py::test_process_crs_pyproj_proj
hvplot/tests/testutil.py::test_process_crs_platecarree[+init=epsg:4326]
hvplot/tests/testutil.py::test_process_crs_mercator[+init=epsg:3857]
/Users/runner/miniconda3/envs/test-environment/lib/python3.11/site-packages/pyproj/crs/crs.py:141: FutureWarning: '+init=<authority>:<code>' syntax is deprecated. '<authority>:<code>' is the preferred initialization method. When making the change, be mindful of axis order changes: https://pyproj4.github.io/pyproj/stable/gotchas.html#axis-order-changes-in-proj-6
in_crs_string = _prepare_from_proj_string(in_crs_string)
'PROJCS["WGS 84 / Pseudo-Mercator",GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","6326"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4326"]],PROJECTION["Mercator_1SP"],PARAMETER["central_meridian",0],PARAMETER["scale_factor",1],PARAMETER["false_easting",0],PARAMETER["false_northing",0],UNIT["metre",1,AUTHORITY["EPSG","9001"]],AXIS["Easting",EAST],AXIS["Northing",NORTH],EXTENSION["PROJ4","+proj=merc +a=6378137 +b=6378137 +lat_ts=0 +lon_0=0 +x_0=0 +y_0=0 +k=1 +units=m +nadgrids=@null +wktext +no_defs"],AUTHORITY["EPSG","3857"]]', | ||
# Created with pyproj.CRS("EPSG:3857").to_wkt() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be nice to still have this comment.
Co-authored-by: Simon Høxbro Hansen <[email protected]>
Closes #1137
The issue was that rioxarray gets loaded after running the second cell, which triggered the first if clause, but we didn't handle WKT strings properly; this PR fixes that.
Supersedes contaminated #1138
Note the notebook cell execution number