ProjectPythia / landsat-ml-cookbook

Machine learning on Landsat satellite data using open source tools
https://projectpythia.org/landsat-ml-cookbook/
Apache License 2.0
11 stars 4 forks source link

Intake v2 #22

Closed ahuang11 closed 6 months ago

ahuang11 commented 7 months ago
review-notebook-app[bot] commented 7 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

github-actions[bot] commented 7 months ago

👋 Thanks for opening this PR! The Cookbook will be automatically built with GitHub Actions. To see the status of your deployment, click below. 🔍 Git commit SHA: 55a06c3bf1791622b9ea063708c6294a1d4daa43 ✅ Deployment Preview URL: In Progress

droumis commented 7 months ago

Any further additions from Martin's revision?

ahuang11 commented 7 months ago

Yes, I'm not sure if Martin's require a new version of intake or not

ahuang11 commented 6 months ago

Added stuff from https://github.com/ProjectPythia/cookbook-template/issues/160

droumis commented 6 months ago

@ahuang11 I'm seeing Panel/Bokeh error in every notebook in the preview https://projectpythia.org/landsat-ml-cookbook/_preview/22/notebooks/1.1_Data_Ingestion-Intake.html

But I'm not seeing this error when I run the notebook locally with an env created with the environment.yml file, which results in: bokeh 3.3.4 panel 1.3.8

droumis commented 6 months ago

as soon as these errors are resolved, let's merge

ahuang11 commented 6 months ago

What version of intake do you have?

droumis commented 6 months ago

intake 2.0.3

ahuang11 commented 6 months ago

The errors will be resolved when https://github.com/intake/intake/pull/791 is released.

ahuang11 commented 6 months ago

Okay new issues that I'll have to look into later..

image

Probably have to completely refactor Preprocessing - Regrid as the dataset likely doesn't exist anymore, or requires a AWS key.

image
droumis commented 6 months ago

@sandhujasmine do you remember where this dataset might have been moved to? I think I remember you working to host it elsewhere at some point (maybe?).

droumis commented 6 months ago

@ahuang11, I'm not sure much if any of the regrid notebook makes sense anymore if someone is going to use the planetary computer data source. As described at the top of it, the regrid notebook was intended as a continuation of the original intake notebook which required additional processing before analysis because it loaded from a different data source. Now that the new intake notebook is just grabbing data from planetary computer, the intake notebook is kind of just a side note about another way to load from the same data source. Although it was a pretty big effort to create, if you don't think there is anything useful left in the regrid notebook then I'd maybe vote to just remove it.

sandhujasmine commented 6 months ago

@sandhujasmine do you remember where this dataset might have been moved to? I think I remember you working to host it elsewhere at some point (maybe?).

Hi @droumis - I did move the landsat data to the holoviz s3 bucket; see this PR: https://github.com/holoviz-topics/examples/pull/274/files - I think this is the correct dataset but let me know if I misunderstood.

ahuang11 commented 6 months ago

Wooh the intake notebook successfully ran in the action; will investigate the other notebook tomorrow.

image
ahuang11 commented 6 months ago

So one other thing I wanted to add + demo was to_yaml_file and open_catalog but currently experiencing issues exporting. At the moment to move this forward, I think we could add it in a separate PR and delete 2.0 regrid notebook?

droumis commented 6 months ago

@ahuang11 ping me when you want me to take a look at any of your changes! I'm guessing that the build error is preventing the 'Deployment Preview URL' .