Remove Tag from Hazard #767
Conversation
|
Looks good to me! Can you please also update the changelog? |
| hf_data.get(var_name)) | ||
| elif var_name == 'tag': | ||
| # legacy behavior: haz_type used to be part of the hazard tag | ||
| if "haz_type" in hf_data.keys(): |
There was a problem hiding this comment.
Will this not make it impossible to read files created with previous versions of CLIMADA as the haz_type cannot be loaded?
There was a problem hiding this comment.
Good question! And if so it should show in the integration test, shouldn't it?
There was a problem hiding this comment.
If the integration tests would be complete yes. But they are not. Ideally, we should write one now. (actually a unit test).
There was a problem hiding this comment.
Actually it's not a problem: the hazard type is read below in lines
elif isinstance(var_val, np.ndarray) and var_val.ndim == 1:
hazard_kwargs[var_name] = np.array(hf_data.get(var_name))(file_name, description and haz_type have always been attributes of the hdf5 dataset itself and never of any tag dataset beneath it)
There was a problem hiding this comment.
The test for it is climada.test.test_api_client.TestClient.test_get_hazard.
climada/hazard/base.py
Outdated
| try: | ||
| comment = u_hdf5.get_string(data[var_names['var_name']['comment']]) | ||
| attrs["description"] += ' ' + comment |
There was a problem hiding this comment.
What is this attribute description? Is this not part of the tag?
There was a problem hiding this comment.
👍 I'll remove it as well.
peanutfun
left a comment
There was a problem hiding this comment.
Looks good to me, nice work! What about the failing compatibility test? The adjustment on the Pythonpath is gone from the changes, somehow.
Changes proposed in this PR:
This PR fixes #
PR Author Checklist
develop)PR Reviewer Checklist