Great updates! I am however not convinced yet by the change

There were many places where it was not entirely obvious whether impact or insured_impact should be used. Therefore the splittin of methods has been (partly) reversed and impact got an additional argument, insured, which is derived from the exposures object if it is given None.

I think this warrants a more in-depth discussion. The variable insured is not clear in this context to me at first sight. Also, the solution lacks some elegance, since now there are two ways to compute the same thing.

This is mostly a fix to ensure backward compatibility, right? How about instead we right a method check_exposure_insured which can be executed before an impact computation (and which also to choose either method?). It is a similar solution, but I think this might be cleaner.

Also, the solution lacks some elegance, since now there are two ways to compute the same thing.

Only formally. insured_impact is nothing but impact(insured=True), stated so in the pydoc string and implemented that way. But I'm happy with removing it altogether.

How about instead we right a method check_exposure_insured which can be executed before an impact computation

You mean instead of using a None - default set the default to False (impact(insured=False)) and call it everywhere either so:

impact = impactCalc.impact() if impactCalc.check_exposure_insured() else impactCalc.insured_impact()

or this way:

impact = impactCalc.impact(insured=impactCalc.insured_impact())

Don't know. Looks noisy to me. The method itself sure seems neat but being forced to call it all the time doesn't.

My suggestion was rather to reverse the change completely. Instead, create a method check_if_insured(exp). Then, the code would be where needed for backward compatibility:

if check_if_insured(exposures):
    impact = impactCalc.insured_impact()
else:
    impact = impactCalc.impact()

In this way, the results from the previous impact.calc() would be reproduced, but it would make it explicit that the calculated impact is the insured loss, and not the residual loss or the total loss. The goal of having the method insured_impact() is precisely to resolve the ambiguity of whether the insured or the non-insured impact is wanted.

Am I understanding you right, on the bottom line you prefer to have no default behavior at all?
Do we know the explicit "insuredness" for all the impact calculations from measures, calibration, lines_polygons, unsequa, cost_benefit, etc?
If not - would there be a default behavior in these, "more advanced", modules but none for the common case?
Or would we need to introduce distinct methods for all methods calling ImpactCalc.impact?

I would like to have a default behaviour that is explicit (it is not dependent on the presence or absence of data in exposures). The default behavior is to compute impacts (which is what ImpactCalc.impact() does). In case someone wants the non-default insured impacts, then she should use ImpactCalc.insured_impact().

Now, this is a change from the previous behavior, where if Exposures.gdf.cover or Exposures.gdf.attachment are defined, then the result is insured impacts, and if they are not defined it is simple impacts. I prefer this, since now it is possible for a user to compute both the impact and the insured impact from the same exposure, without having to manipulate the exposure. Furthermore, it prevents users from obtained insured impacts, even thought they meant to get impacts.

But, your question if I understand it correctly is that in the other modules meaures, calibration, lines_polygons, unsequa and cost_benefit, the current default is impact.calc and it is not clear whether this should be replaced by ImpactCalc.impact or by ImpactCalc.insured_impact or something else?

the current default is impact.calc and it is not clear whether this should be replaced by ImpactCalc.impact or by ImpactCalc.insured_impact or something else?

Exactly.

I prefer this, since now it is possible for a user to compute both the impact and the insured impact from the same exposure, without having to manipulate the exposure.

That hasn't changed with this PR. Still possible.

I would like to have a default behaviour that is explicit (it is not dependent on the presence or absence of data in exposures). The ...

When I say default behavior I mean default behavior or the program not the user 😄

I would like to have a default behaviour that is explicit (it is not dependent on the presence or absence of data in exposures). The ...

When I say default behavior I mean default behavior or the program not the user 😄

:D what I mean is that the default behaviour imho should be to compute impacts, no under-the-hood insured impacts. But, for what should be used in other modules, I think warrants a deeper discussion. Let us take it to the developer's meeting next week ok?

Updates:

• ImpactCalc.insured_impact has been removed.
• ImpactCalc.impact takes two new flag-arguments in lieu of the forsaken insured: ignore_cover and ignore_deductible