Feature/coastal flooding #100
Conversation
There was a problem hiding this comment.
Cool addition!
@emanuel-schmid : can you please just check how we should handle the external resources (here a weblink)? In particular, should we do something for the case that the link changes of the data is no longer available?
| def __init__(self, **kwargs): | ||
| """Empty constructor""" | ||
|
|
||
| Hazard.__init__(self, HAZ_TYPE, **kwargs) | ||
|
|
||
| @classmethod | ||
| def from_aqueduct_tif(cls, |
There was a problem hiding this comment.
In this case I think it makes much more sense to remove the from method and put all the functionality in the init. There seems to be no other purpose than to load data (initialize the object).
There was a problem hiding this comment.
mhh, as of now maybe yes, but it's likely that the CoastalFlood class will be populated with more data in the future; same as it's happening with the RiverFlood class where we now have ISIMIP and Aqueduct (https://github.com/CLIMADA-project/climada_petals/blob/feature/river_flooding_aqueduct/climada_petals/hazard/river_flood.py) so I would tend to keep it like this.
|
Any update on this: @emanuel-schmid : can you please just check how we should handle the external resources (here a weblink)? |
| # FIX THIS PATH - YOU SHOULD SAVE UNDER HAZARD, NOW IT CREATES A FOLDER CALLED COASTALFLOOD | ||
| # AT THE SAME LEVEL AS HAZARD. RIVERINE AQUEDUCT IS SAVED THERE TOO, SO FIX THAT TOO | ||
| # (SEE CLIMADA DATA LOCAL FOLDER) |
There was a problem hiding this comment.
Is this comment still valid? Actually the path ~/climada/data/hazard is where the files from the data api land. There are some automatic features on this directory executed by the api client: caching, making sure that the version is always the newest and purging. So, strictly speaking, the data files that are not handled by the climada api client should not necessarily be saved there. Any other directory on the same level actually seems better suited.
Hence I'd rather remove the comment than change the config.
|
Any update on this @aleeciu and @emanuel-schmid ? |
|
Hi @chahank , meanwhile @emanuel-schmid and I actually made these data accessible via the API. So I wonder if there is any point in completing this PR in the first place. All it does is loading data that are now accessible via the API. |
What is your take on this @chahank @emanuel-schmid ? |
|
Imho, just because the resulting data is available elsewhere doesn't mean that the code is obsolete.
|
|
Given that we clarified the issue with the config file, from my end this PR is complete. As for #153 I seem to be getting test errors about wind calculation. I don't know where this comes from. |
Changes proposed in this PR:
PR Author Checklist
develop)PR Reviewer Checklist