momepy.COINS: raise ValueError if empty geodataframe is passed

anastassiavybornova commented 1 month ago

Description / Summary

suggeston is to add a ValueError to be raised if the argument passed to momepy.COINS (edge_gdf) is empty

Value / benefit

Would make debugging easier. Currently the code breaks a bit further down the line (see below); I was passing an empty gdf by mistake and it took me a while to figure out where the issue came from. For example:

import momepy
momepy.COINS(
    edge_gdf = gpd.GeoDataFrame(
        geometry = [],
        crs="EPSG:4326"
    )
)

currently gives:

---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
Cell In[13], [line 2](vscode-notebook-cell:?execution_count=13&line=2)
      [1](vscode-notebook-cell:?execution_count=13&line=1) import momepy
----> [2](vscode-notebook-cell:?execution_count=13&line=2) momepy.COINS(
      [3](vscode-notebook-cell:?execution_count=13&line=3)     edge_gdf = gpd.GeoDataFrame(
      [4](vscode-notebook-cell:?execution_count=13&line=4)         geometry = [],
      [5](vscode-notebook-cell:?execution_count=13&line=5)         crs="EPSG:432[6](vscode-notebook-cell:?execution_count=13&line=6)"
      6     )
      [7](vscode-notebook-cell:?execution_count=13&line=7) )

File ~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:99, in COINS.__init__(self, edge_gdf, angle_threshold, flow_mode)
     [96](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:96) self._unique_id()
     [98](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:98) # compute edge connectivity table
---> [99](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:99) self._get_links()
    [101](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:101) # find best link at every point for both lines
    [102](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:102) self._best_link()

File ~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:174, in COINS._get_links(self)
    [171](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:171) self.temp_array = np.array(self.temp_array, dtype=object)
    [173](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:173) items = collections.defaultdict(set)
--> [174](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:174) for i, vertex in enumerate(self.temp_array[:, 1]):
    [175](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:175)     items[vertex].add(i)
    [176](https://file+.vscode-resource.vscode-cdn.net/Users/anvy/Library/CloudStorage/OneDrive-ITU/projects/bike-prior/~/anaconda3/envs/bikeprior/lib/python3.12/site-packages/momepy/coins.py:176) for i, vertex in enumerate(self.temp_array[:, 2]):

IndexError: too many indices for array: array is 1-dimensional, but 2 were indexed

Implementation details

No response

Tasks to complete

happy to make a PR - if that's the behaviour we want to have? also, since i'm at it -- any other checks of input data we want to have?

martinfleis commented 1 month ago

That's user's responsibility in my opinion. We would have to do this everywhere.

anastassiavybornova commented 1 month ago

makes sense, then i hereby accept my responsibility and close this issue again :D

pysal / momepy