upgrade_shapely_2.0_fix

[emanuel-schmid]

Changes proposed in this PR:

• use the .geoms attribute as mask argument in case of MultiPolygon

This PR fixes CLIMADA-project/climada_petals#79

PR Author Checklist

• Read the Contribution Guide
• Correct target branch selected (if unsure, select develop)
• Descriptive pull request title added
• Source branch up-to-date with target branch
• Documentation updated
• Tests updated
• Tests passing
• No new linter issues
• Changelog updated

PR Reviewer Checklist

• Read the Contribution Guide
• CLIMADA Reviewer Checklist passed
• Tests passing
• No new linter issues

[tovogt]

I think that there is a more fundamental issue here.

The read_raster function takes an argument geometry that is passed on to rasterio.mask.mask as the shapes argument. The docstring of read_raster specifies the type of the geometry to be a shapely.geometry, but the docstring of rasterio.mask.mask specifies the type of shapes to be an iterable of geometries.

This was never correct, even before the 2.0 upgrade for shapely.

The tests for read_raster are currently passing a list of geometries so that the tests don't fail even though the tests aren't according to the docstring:

climada_python/climada/util/test/test_coordinates.py

Line 1265 in 4d8fa43

meta, inten_ras = u_coord.read_raster(HAZ_DEMO_FL, geometry=[poly])

I suggest to rename the kwarg to geometries and correctly specify its type as list of shapely.geometries in the docstring. Then go through the core and petals code base and replace the calls to read_raster accordingly. There are only a few places that are affected by this. Furthermore, I don't think that we need to have backwards compatibility here, because my impression is that the read_raster function is so overwhelming that almost nobody uses it outside of the CLIMADA code.

Note that this issue also affects the Exposures.from_raster functionality since the arguments are basically passed on to the read_raster function:

climada_python/climada/entity/exposures/base.py

Lines 463 to 480 in 4d8fa43

	def from_raster(cls, file_name, band=1, src_crs=None, window=False,
	geometry=False, dst_crs=False, transform=None,
	width=None, height=None, resampling=Resampling.nearest):
	"""Read raster data and set latitude, longitude, value and meta
	Parameters
	----------
	file_name : str
	file name containing values
	band : int, optional
	bands to read (starting at 1)
	src_crs : crs, optional
	source CRS. Provide it if error without it.
	window : rasterio.windows.Windows, optional
	window where data is
	extracted
	geometry : shapely.geometry, optional
	consider pixels only in shape

I would still claim that we don't need backwards compatibility for the geometry argument in the Exposures.from_raster function, and propose to change this argument to geometries, as well.

[emanuel-schmid]

@tovogt Excellent! 🥇 Many thanks for the profound analysis. Your suggestions make a lot of sense. I think I'm just going to implement them.

[emanuel-schmid]

I've reversed the check for MultiPolygon and made it officially the responsibility of the caller to make sure the geometry argument is a list of shapely.geometry. That is, I updated the doc strings, but not the methods.
This is now the case for 5 related methods.

While at it, I changed the default values for many arguments in these methods to None where False seemed less appropriate.

I didn't change the argument names, reluctant to alter the signature without being forced to. 🤷

Bottom line: nothing has changed, the PR is reduced to cosmetics.

[tovogt]

I would have a tiny suggestion how to clean up the code. But since it only affects the tests, it's not really important. The rest is fine with me.

[tovogt]

Thanks, looks good!