replace `cellid` by `icell2d` (or `row` and `col`) in result of `nlmod.grid.gdf_to_grid`

[rubencalje]

Right now nlmod.grid.gdf_to_grid adds a column to the resulting GeoDataFrame called cellid. This column contains an integer (the icell2d-number) for vertex models, and a tuple of two integers (the row and column) for structured grids.

I think cellid is a badly chosen name. Generally, in MODFLOW cellid refers to the combination of layer and icell2d (or layer, row and column). Also, for structured grids, the tuple in the cellid-column can cause problems. For example, when saving the resulting GeoDataFrame to a geojson-file, this tuple is converted to a string.

Therefore I am in favor of replacing the column cellid by icell2d in the result of nlmod.grid.gdf_to_grid for vertex grids, and by a column named row and a column named col for structured grids.

This change unfortunately will cause a lot of problems in people's scripts, so I think the default for now would be to keep the current behavior, and generate a warning that things will change in the future. People can turn off this warning by setting the parameter use_cellid. So for example, to keep the current behavior:

gdf_grid = nlmod.grid.gdf_to_grid(gdf, ds, add_cellid=True)

Or adopt to the new behavior:
gdf_grid = nlmod.grid.gdf_to_grid(gdf, ds, add_cellid=False)

What do you think?

[dbrakenhoff]

I used cellid in flopy's GridIntersect, which is probably why it got named that way in nlmod. I guess I took the term cellid to just mean some general form of grid address, and not necessarily requiring a complete address. But I agree it would be nice to be very consistent across nlmod with these types of things.

I'm fine with the addition of the "row", "col" or "icell2d" columns to the dataframe. That even saves you a small step from having to extract the rows and columns into separate variables. The "cellid" result had to be handled differently for structured or vertex grids anyway, so I don't think having different column names with the output changes the complexity of the code any.

So +1 on implementing that and leaving the cellid in the results with a warning, that can be silenced by use_cellid or add_cellid.

[bdestombe]

+1. Only I would suggest to rename the flag to something more verbose: use_icellid_instead_of_icellid2d