Skip to content

add gdf_zonal_stat to main.py#293

Open
TomKenda wants to merge 3 commits into
perrygeo:masterfrom
TomKenda:patch-1
Open

add gdf_zonal_stat to main.py#293
TomKenda wants to merge 3 commits into
perrygeo:masterfrom
TomKenda:patch-1

Conversation

@TomKenda
Copy link
Copy Markdown

Hello,
I am not sure it's the best way to do this but I would like to propose a function to get the result of the zonal stat directly as a Geodataframe to keep all the properties and the geometry together with the initial vectors CRS. I you think there's a more efficient way to do this I would be happy to ear it.

here is a function to get the result of the zonal stat directly as a Geodataframe to keep all the properties and the geometry together with the initial vectors CRS. I you think there's a more efficient way to do this I would be happy to ear it.
Copy link
Copy Markdown
Owner

@perrygeo perrygeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea for a user experience improvement. I'd like to see a test but we can add that later.

Comment thread src/rasterstats/main.py Outdated
"""

gdf = gpd.GeoDataFrame.from_features(
list(gen_zonal_stats(vectors, geojson_out=True, *args, **kwargs))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from_features should be able to take an iterable sequence - we could remove the intermediate list.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, I have just tried without the list and it works !

function `list` was not necessary
@TomKenda
Copy link
Copy Markdown
Author

TomKenda commented Oct 5, 2023

Good idea for a user experience improvement. I'd like to see a test but we can add that later.

Are you talking about unit test or just an example ?

@perrygeo
Copy link
Copy Markdown
Owner

perrygeo commented Oct 5, 2023

A unit test would be ideal. Geopandas is included in the CI test runner so it should work.

@TomKenda
Copy link
Copy Markdown
Author

I am not sure I did it the right way but I added a unit test and updated the function to fit to a file path input

Comment thread tests/test_zonal.py
import sys

import numpy as np
import geopandas as gpd
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see in the test you runned that there are problems with geopandas import. I don't know what to do to solve this. Maybe I was not supposed to add this line to the test file ?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using gpd = pytest.importorskip("geopandas") in your test functions should be enough. If it's available, you get gpd, otherwise the test is skipped. You don't want to import it at the top level.

@TomKenda TomKenda closed this Sep 3, 2025
@TomKenda TomKenda reopened this Sep 3, 2025
Comment thread tests/test_zonal.py
import sys

import numpy as np
import geopandas as gpd
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using gpd = pytest.importorskip("geopandas") in your test functions should be enough. If it's available, you get gpd, otherwise the test is skipped. You don't want to import it at the top level.

Comment thread tests/test_zonal.py

#%% # add a unit test for the function gdf_zonal_stats

def gdf_zonal_stats(vectors, *args, **kwargs):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a copy of what's in main.py. Why duplicate it instead of importing it?

Comment thread src/rasterstats/main.py
Besides this, we make sure the new gdf as the right
metadata by conserving the CRS
"""

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the import geopandas as gpd into the function body and out of the top level. Geopandas must remain optional.

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.

2 participants