Comments on accessing s3 #10
Conversation
| @@ -110,15 +115,26 @@ | |||
| "metadata": {}, | |||
| "outputs": [], | |||
| "source": [ | |||
| "dc1 = xr.open_dataset(url1, engine= 'zarr',\n", | |||
| "dc1 = xr.open_dataset(url1, engine= 'zarr', chunks=\"auto\",\n", | |||
There was a problem hiding this comment.
This makes the repr faster. I'll open an xarray issue
accessing_s3_data.ipynb
Outdated
| @@ -127,11 +143,8 @@ | |||
| "metadata": {}, | |||
| "outputs": [], | |||
| "source": [ | |||
| "def get_bbox_single(input_xr):\n", | |||
| "def get_bounds_polygon_4326(input_xr):\n", | |||
There was a problem hiding this comment.
refactoring this piece out simplifies the code later
| "\n", | ||
| " #format question - better to condense this into 1 line or break into 3 to be more readable?\n", | ||
| " http_url = input_dict['features'][granule]['properties']['zarr_url']\n", | ||
| " for granule in input_dict['features']:\n", |
There was a problem hiding this comment.
note how using python's iterating feature makes the code simpler and more readable.
| " ### alternatively use list comprehension\n", | ||
| " # poly_ls = [get_bounds_polygon(xr_obj).to_crs('epsg:4326') for xr_obj in input_ls]\n", |
There was a problem hiding this comment.
The following pattern
out = []
for a in b:
...
out.append(...)
is usually a good place to use a list comprehension.
| @@ -449,7 +468,7 @@ | |||
| "metadata": {}, | |||
| "outputs": [], | |||
| "source": [ | |||
| "get_bbox_group(dc_ls, [70, 98, 20, 60])" | |||
| "get_bbox_group(dc_ls, bounds=[70, 98, 20, 60])" | |||
There was a problem hiding this comment.
Writing out bounds= is nice because then future-you knows what that list is supposed to mean without looking up the function.
You could enforce this by changing the signature to
def get_bbox_grouop(input_xr, *, bounds=[...])With this you can only set bounds by writing out bounds=.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
This notebook is great. I just have minor comments on increasing readabillity