search

ruithnadsteud / yt_georaster

A package for handling geotiff files and georeferenced datasets within yt.
1 stars 2 forks source link

Milestone 1 #13

Closed arevill2 closed 3 years ago

arevill2 commented 3 years ago

Updates from Dan's feedback.

brittonsmith commented 3 years ago

@arevill2, this looks great. I have two primary points of feedback.

  1. Before we show the timing comparison, I think it's worth showing in more detail what a yt data container is and how you get data from it. Just after the cell where you define get_centroids_XY, I recommend adding a cell where we create a single rectangular data container (say for Edinburgh) and then query the ('bands', '1') field to show how one gets data from it. Then, it would be good to show querying some spatial fields like 'x', 'y', and 'radius'. This is also a good place to show the 'pixels' unit conversion. Then, I would show the projection image of the Edinburgh rectangle here instead of after the timing. This should help set the stage for then showing the timing to show that none of this takes much more time than the rasterio way.
  2. In the cell near the end about "additional data containers", that's a good place to mention milestone 3 which is to implement a data container for arbitrary polygons described by a series of points.

Minor point: in the timing cell, you can replace for example point_center[0:2,0] with point_center[:,0]. Just a little cleaner.