Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-authored-by: Chahan M. Kropf <chahan.kropf@usys.ethz.ch>

@chahank There is another issue: Impact.from_eih now has an unused argument impfset: https://github.com/CLIMADA-project/climada_python/pull/743/files#diff-1c276908f04d1a0ca5b1692295a586ce54b0937673711203582d3d72099ecdd0R210

The impfset was only used to populate the tag. I propose to deal with this the following way:

* Remove the `impfset` parameter from the method immediately

* Update all uses of `from_eih` within Climada (there are not many)

* Add a type check for the second argument. If it is an `ImpactFuncSet`, raise an error with useful error message

This way, we are not backward compatible but users know how to fix the issue.

Excellent idea! Although I think we do not need to make the error message. Most users will not directly use this method anyway imho.

Oh, and probably you should go through the documentation and check whether the tag is explicitly mentioned. If yes, please remove it.

@chahank I removed the impfset parameter from Impact.from_eih without adding an type check, as discussed. I removed the mention of Tag from the impact tutorial. It is still mentioned in other tutorials (e.g. exposures and hazard) though, but I would remove these instances once tags are removed from the respective classes.

Excellent, looks good to me!

Thanks for the input, merging now!