You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Centroids.set_geometry_points is now missing in develop without deprecation warning #884
Problem:
I wanted to rerun a previously working code (e.g. on the current main branch) using the develop branch and it suddenly failed.
import xarray as xr
import numpy as np
import datetime as dt
from climada.hazard import Hazard
"""Write a simple NetCDF file to read"""
netcdf_path = "default.nc"
intensity = np.array([[[0, 1, 2], [3, 4, 5]], [[6, 7, 8], [9, 10, 11]]])
time = np.array([dt.datetime(1999, 1, 1), dt.datetime(2000, 1, 1)])
latitude = np.array([0, 1])
longitude = np.array([0, 1, 2])
dset = xr.Dataset(
{
"intensity": (["time", "latitude", "longitude"], intensity),
},
dict(time=time, latitude=latitude, longitude=longitude),
)
dset.to_netcdf(netcdf_path)
hazard = Hazard.from_xarray_raster_file(netcdf_path, "", "")
hazard.centroids.geometry
# output:
# GeoSeries([], dtype: geometry)
hazard.centroids.set_geometry_points() # <- this line fails in current develop branch
hazard.centroids.geometry
# output:
# 0 POINT (0.00000 0.00000)
# 1 POINT (1.00000 0.00000)
# 2 POINT (2.00000 0.00000)
# 3 POINT (0.00000 1.00000)
# 4 POINT (1.00000 1.00000)
# 5 POINT (2.00000 1.00000)
# dtype: geometry
Possible solution:
Have you considered keeping an "empty" set_geometry_points method instead of just removing it and issuing a deprecated warning informing the user that this function is now useless?
The code would run through smoothly and the result would be correct because now the geometry is already correctly set automatically...
Alternative solution:
I assume the next release would be a major release, so maybe keeping such functionality is not important?
We tried to deprecate methods that should not be used anymore, and immediately removed methods that have no use anymore. However, for methods like set_geometry_points we could indeed retain a stub that does nothing and issues a deprecation warning. We will go through this again and update Centroids accordingly.
Indeed, the next release will be a major, breaking release for these reasons.
Looking at the proposed changes in #587 I fail to see why this method would actually be needed. My understanding is that #857 is trying to use the now-refactored and deprecated raster and pixel logic. But I do not see why this would be necessary for achieving the goal of #857 which to allow the computation, visualization and storage of local frequency exceedance intensities of hazards. So, we might first try to bring #857 up to date with the refactored centroids, and then see whether Centroids.calc_pixels_polygons is needed.
Activity
peanutfun commentedon May 24, 2024
Indeed. During the refactoring of
Centroids, many methods have been removed, see https://github.com/CLIMADA-project/climada_python/blob/develop/CHANGELOG.md#removedWe tried to deprecate methods that should not be used anymore, and immediately removed methods that have no use anymore. However, for methods like
set_geometry_pointswe could indeed retain a stub that does nothing and issues a deprecation warning. We will go through this again and updateCentroidsaccordingly.Indeed, the next release will be a major, breaking release for these reasons.
peanutfun commentedon May 24, 2024
Methods for which we could provide deprecation warnings:
@chahank
peanutfun commentedon Jun 14, 2024
Centroids.calc_pixels_polygonsis used in #857 and might need to be re-introduced.chahank commentedon Jun 14, 2024
Looking at the proposed changes in #587 I fail to see why this method would actually be needed. My understanding is that #857 is trying to use the now-refactored and deprecated raster and pixel logic. But I do not see why this would be necessary for achieving the goal of #857 which to allow the computation, visualization and storage of local frequency exceedance intensities of hazards. So, we might first try to bring #857 up to date with the refactored centroids, and then see whether
Centroids.calc_pixels_polygonsis needed.