Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unified_data_layers.ipynb #6

Merged
merged 2 commits into from
Jun 29, 2021
Merged

Fix unified_data_layers.ipynb #6

merged 2 commits into from
Jun 29, 2021

Conversation

DahnJ
Copy link
Contributor

@DahnJ DahnJ commented Jun 3, 2021

Hi, I'm in the process of providing a streamlined version of unified_data_layers for H3-Pandas. This notebook is actually really useful for me, as it's a great non-trivial use case and informs what the API should look like! However, it was also broken, so I did my best to fix it without changing the functionality.

You can view the notebook in nbviewer

Commit messages have more info:

    Add dependencies to requirements
    
    rtree is required for spatial join
    xarray + rasterio for handling raster data without gdal_translate
    Refactor notebook, remove gdal_translate
    
    Problems solved
    - Outdated/broken code
    - Images displayed without running the code
    - No usage of gdal_translate, which didn't work in Binder
      (at least for me)

closes #5

DahnJ and others added 2 commits June 3, 2021 20:12
Problems solved
- Outdated/broken code
- Images displayed without running the code
- No usage of gdal_translate, which didn't work in Binder
  (at least for me)
rtree is required for spatial join
xarray + rasterio for handling raster data without gdal_translate
@CLAassistant
Copy link

CLAassistant commented Jun 3, 2021

CLA assistant check
All committers have signed the CLA.

@DahnJ DahnJ changed the title Fix unified_data_layers.ipynb WIP: Fix unified_data_layers.ipynb Jun 3, 2021
@DahnJ
Copy link
Contributor Author

DahnJ commented Jun 3, 2021

I have just noticed the issues in this repo regarding this notebook, I will check if this solves them, or whether I need to perform more changes.

edit: Okay, this closes #5. There's a valid point in #4 but I didn't want to change the actual functionality of the notebook, so I will keep it like it is. Plus it's not entirely clear to me whether the "smoothing" functions should function as a spatial moving average (as @GpoloVera suggests), or whether this is more of a "diffusion".

@DahnJ DahnJ marked this pull request as draft June 3, 2021 18:41
@DahnJ DahnJ marked this pull request as ready for review June 3, 2021 18:51
@DahnJ DahnJ changed the title WIP: Fix unified_data_layers.ipynb Fix unified_data_layers.ipynb Jun 3, 2021
@ajfriend
Copy link
Contributor

Thanks! Appreciate the fix!

@ajfriend ajfriend merged commit 2088cfb into uber:master Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ValueError: 'left_df' should be GeoDataFrame, got <class 'pandas.core.frame.DataFrame'>
3 participants