CosmiQ / solaris

CosmiQ Works Geospatial Machine Learning Analysis Toolkit
https://solaris.readthedocs.io
Apache License 2.0
414 stars 112 forks source link

[MAINT]: manage CRS objects, part deux #333

Open nrweir opened 4 years ago

nrweir commented 4 years ago

Maintenance request summary

Given the changes being made to geopandas CRS management, we're once again re-examining how CRSs should be managed and passed around within solaris. There are a few options (some discussed here:

  1. Keep it the same (still use rasterio.crs.CRS objects), and just convert to pyproj.CRS objects for geopandas.GeoDataFrames;
  2. Pass around pyproj.CRS objects, and convert them to rasterio.crs.CRS objects when necessary for images;
  3. Pass around proj6 strings or WKT strings or something like that, then convert them to whatever appropriate format for the georegistered object type we're using.
  4. Something else?

My inclination is to go with either 1. or 2. to take advantage of some of the functionality built into those classes, but am open to other opinions. Feel free to add your thoughts, @rbavery or anyone else.

Task detail and notes (to be filled in later)

Any additional information:

nrweir commented 4 years ago

After digging into this a bunch (and trying to figure out how to handle the new version of geopandas best), I think we're going to pass around CRSs as pyproj.CRS objects instead of rasterio. See this gist for an example of why. PR in preparation.

rbavery commented 4 years ago

Hey @nrweir, I'm wrapping up #331 but have hit a snag in the raster_tiler where reprojecting with rasterio doesn't support pyproj CRS objects. In this case, self.dest_crs is pyproj object calculate_default_transform is required for rasterio.warp.reproject.

dst_transform, width, height = calculate_default_transform(
                    self.src.crs, self.dest_crs,
                    self.src.width, self.src.height, *tb,
                    dst_height=self.dest_tile_size[0],
                    dst_width=self.dest_tile_size[1])

I seem to remember there was a discussion somewhere with rasterio folks on supporting pyproj objects but I'm not sure where that issue or where that discussion went. I took a look at your gist and saw that converting between rasterio and pyproj CRS objects doesn't look safe unless the conversion is based on EPSG codes :/ Should we go this route until/if rasterio supports pyProj CRS objects?

nrweir commented 4 years ago

@rbavery - thanks for following up here. yeah, let's go with EPSG codes until we can figure out a way to do generalized pyproj.CRS -> rasterio.crs.CRS interconversion more effectively.