Review Notebooks on Data access, chunking and Dask

pangeo-data / foss4g-2022

Pangeo tutorial at FOSS4G 2022

https://pangeo-data.github.io/foss4g-2022

Other

2 stars 9 forks source link

Review Notebooks on Data access, chunking and Dask #45

Closed guillaumeeb closed 2 years ago

guillaumeeb commented 2 years ago

Creating a draft pull request after the review of the three notebooks on Data and Dask.

Some high level firsts impressions there, and after that I'll try to comment more precisely using github review (or NBReview) later on.

data_discovery is nice. It lacks a bit of computations or exercise, but it's short so probably OK.
chunking_introduction is hard. I try to propose some content modifications, but still the part on kerchunk is complicated. Moreover, I'm not sure if Kerchunk is really needed to explain chunks. It's just a good way to consolidate a dataset and make it easily loadable.
dask_introduction is fine, but it currently lacks real processing (which is perfectly normal as it is probably not finished). I only saw a processing over Lombardia. And as there is currently problem with dask-gateway setup (see https://github.com/pangeo-data/pangeo-eosc/issues/3), it still hard to follow.

review-notebook-app[bot] commented 2 years ago

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

guillaumeeb commented 2 years ago

Okay, I tried to point some interrogations I had about those notebooks.

This is already great work and I know how hard it is to prepare such material, so please don't take my comments badly. I'm only trying to improve this material and makes it as easy as possible for trainees to follow the content.

acocac commented 2 years ago

@guillaumeeb all your comments are great. Thanks for your constructive feedback!

acocac commented 2 years ago

@guillaumeeb the data and discovery notebook is still, I hope to add a minimal example to access a STAC catalog of Sentinel-2 and maybe another example of available pangeo-forge STAC dataset. I'll merge coming changes in the notebook from the main branch to this PR to get your feedback. Thanks

pl-marasco commented 2 years ago

@guillaumeeb please don't even mention that, without good criticism and different eyes there would not be growth.

All my material has been prepared from the scratch and on the fly so there is a lot that is missing and is really good to have different visions, opinions and hints.

annefou commented 2 years ago

There are some conflicts to fix but apart from that it is ok to me. So once the conflicts are resolved we can merge.

guillaumeeb commented 2 years ago

@annefou I resolved the conflict.

I think it's a god idea to merge before things diverge again.

The thing is I wanted to also discuss some points with all of you (and @tinaok did not made any comment yet).

So what I propose is that:

We merge this if everyone agree on the modifications
I open some issues on the most important points to discuss and solve my comments:
- Add more explanation on chunking before introducing Kerchunk and multiple file reading. See if we keep all kerchunk part as is. Talk a bit more about Dask arrays.
- Add some content on compression.
- Do we keep setup section in all notebooks? It's hard to maintain.
- Talk more about Dask in dask_introduction, the Dask clusters and dashboard concepts.
- Remove the part on installing libraries on dask workers (see https://github.com/pangeo-data/pangeo-eosc/issues/3)
- Scale to the whole world on dask_intro.
We can continue to discuss little things here if needed.

Thoughts?

annefou commented 2 years ago

It is easier to merge and make new issues and PR.