CLIMADA-project / climada_python

Python (3.8+) version of CLIMADA
GNU General Public License v3.0
321 stars 125 forks source link

Getting TC from the API client removes some of their attributes #954

Open spjuhel opened 1 month ago

spjuhel commented 1 month ago

Describe the bug I noticed that getting TCs (And I suppose any Hazard) from the API removes some of their attributes (at least basin).

To Reproduce Steps to reproduce the behaviour/error:

from climada.hazard import TCTracks, TropCyclone
from climada.util.api_client import Client
client = Client()

tc_haiti = client.get_hazard('tropical_cyclone', properties={'country_name': 'Haiti', 'climate_scenario': 'rcp45', 'ref_year':'2040', 'nb_synth_tracks':'10'})
tc_haiti.basin

------------------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[41], line 1
----> 1 tc_haiti.basin

AttributeError: 'Hazard' object has no attribute 'basin'
-----------------------------------------------------------------------------------

Expected behavior

The TC attributes are correctly read.

Climada Version: 5.0.1-dev (But presumably also 5.0.0)


My understanding of the problem is that get_hazard() calls Hazard.from_hdf5() (or the appropriate reader) and that the reader methods within Hazard only read Hazard defined attributes and not the ones defined in subclasses. I already noticed something similar when storing/reading hdf5 files within a pipeline (i.e., that the defined attributes were not the same depending on if you read the files with Hazard.from_hdf5() or TropCyclone.from_hdf5()).

The problem for the client.get_hazard() method can probably be easily fixed by using the reader from the appropriate subclass depending on the "hazard_type" argument given to the method.

But I think it would be possible (and much better), for the Hazard reader to read and return the "intended" object from the file. That is Hazard.from_hdf5("Dataset_of_tropical_cyclones.hdf5") would return a TropCyclone and not Hazard. My understanding is that this would require implementing a Factory Pattern, with inspiration from here, for instance.

This would probably be a sizeable overall of the reader (and maybe the writers also), but I think this is something we should have in mind.