fix hardcoded text elements in the plots of the forecast class

[ThomasRoosli]

Changes proposed in this PR:

• fix hardcoded text elements in the plots of the forecast class

This PR fixes #

PR Author Checklist

• Read the Contribution Guide
• Correct target branch selected (if unsure, select develop)
• Descriptive pull request title added
• Source branch up-to-date with target branch
• Documentation updated
• Tests updated
• Tests passing
• No new linter issues
• Changelog updated

PR Reviewer Checklist

• Read the Contribution Guide
• CLIMADA Reviewer Checklist passed
• Tests passing
• No new linter issues

[timschmi95]

Looks good to me. There are two variable names still containing wind (wind_map_file_name, wind_map_file_name_full) which could also be adjusted.

[timschmi95]

Looks good to me. I didn't realize the split was because of the linter. I guess the previous version would've been also good then (maybe better for some), although I personally still like the new version better.

[peanutfun]

I find a lot of inconsistencies in the code of these plots. But since we want to overhaul the entire class and make it more useful until then without changing any of its "logic", I think it's fine.

Are you sure about the wordings, though? I find figure titles usually unnecessary. The default "explain text" states what I would use as a colorbar label, e.g. "Mean Wind Damage" or "Total Building Damage". There is no need to repeat this information in the title, imho.

[peanutfun]

Since there is quite some fiddling with titles, it might make sense to assert if the correct figure title is set with something like

ax = forecast.plot_exceedence_prob(explain_str="Something from the title")
self.assertIn(ax.get_title(), "Something from the title")

[ThomasRoosli]

I find a lot of inconsistencies in the code of these plots. But since we want to overhaul the entire class and make it more useful until then without changing any of its "logic", I think it's fine.

Are you sure about the wordings, though? I find figure titles usually unnecessary. The default "explain text" states what I would use as a colorbar label, e.g. "Mean Wind Damage" or "Total Building Damage". There is no need to repeat this information in the title, imho.

I agree with waiting to improve the plots when we do the overhaul of the entire class.

About the need of titles: as there is a lot of forecast specific information in the plot titles (actually one of the features of the class), I find using the title to quickly spot the type of plot quite useful. But luckily you don't have to agree, because from now on you can assign an empty string to the "explain text" to have no repetition of the colorbar label.

[peanutfun]

@ThomasRoosli True! 😁 Could you have a look at my suggestions and comment/commit/adapt them at your convenience? Then we are ready to merge!

[ThomasRoosli]

@peanutfun I added the suggested functionality in the tests and additionally some more asserts and structure.
Further test upgrades can happen in the overhaul of the module. Can you have a look at it again?

[peanutfun]

Looks very good now, thanks for improving the new tests! I'll see ich the pipeline succeeds and merge afterwards