Docs still showcase deprecated methods and new methods switch away from in-place assignment without warning

[riley-brady]

This one took a bit to debug! Note that the climada-petals documentation uses deprecated methods for setting up e.g. Wildfire as a hazard. They use WildFire.set_hist_fire_FIRMS, which leads to a deprecation warning asking to use WildFire.get_hist_fire_FIRMS. There is no note in the docs, docstrings, or warning that expected behavior changes away from in-place assignment.

The following from the tutorial (https://climada-petals.readthedocs.io/en/latest/tutorial/climada_hazard_Wildfire.html) works:

from climada.util.constants import DEMO_DIR
from climada_petals.hazard import WildFire
import os

d_path = os.path.join(DEMO_DIR, "Portugal_firms_June_2017.csv")
firms = pd.read_csv(d_path) 
wf_pt = WildFire()
# Note that in-place assignment is encouraged here from docs and works.
wf_pt.set_hist_fire_FIRMS(firms, centr_res_factor=1./2.5)
wf_pt.plot_intensity(event=0)

You get the following deprecation warning:

climada_petals.hazard.wildfire - WARNING - The use of WildFire.set_hist_fire_FIRMS is deprecated.Use WildFire.from_hist_fire_FIRMS 

Okay, so we check out the wf_pt.from_hist_fire_FIRMS docstring and it's quite similar it seems. So we try the following, just changing the method:

wf_pt = WildFire()
# Using in-place assignment again
wf_pt.from_hist_fire_FIRMS(firms, centr_res_factor=1./2.5)
wf_pt.plot_intensity(event=0)

This leads to the following cryptic error:

ValueError: zero-size array to reduction operation

I noticed in the code, you are actually returning something for from_hist_fire_FIRMS, so let's try with re-assignment of variable:

wf_pt = WildFire()
# Re-assign class here
wf_pt = wf_pt.from_hist_fire_FIRMS(firms, centr_res_factor=1./2.5)
wf_pt.plot_intensity(event=0)

Aha! This works! But note if you try re-assignment with the deprecated method, you get an error since it doesn't return an object:

wf_pt = WildFire()
# Try re-assignment with old method?
wf_pt = wf_pt.set_hist_fire_FIRMS(firms, centr_res_factor=1./2.5)
wf_pt.plot_intensity(event=0)

This leads unsurprisingly to:

AttributeError: 'NoneType' object has no attribute 'plot_intensity'

I would suggest a few things to help alleviate issues for new users:

1. Update the tutorial documentation to use the new methods, since we'll get deprecation warnings currently and end up trying to replace this in the code. Make sure to drop the in-place assignment in the tutorial pages.
2. For the deprecation warning, I would add a note that you've moved away from in-place assignment.
3. In the docstrings for the new methods, such as get_hist_fire_FIRMS, I would add a clear note of the changing behavior

"""Docstring

    NOTE: Behavior has changed from the old `set_hist_fire_FIRMS`. In-place assignment no longer works, and the user is required to use variable reassignment.

Parameters
----------
blah blah

Returns
-------
blah blah

Examples
--------
> from climada.util.constants import DEMO_DIR
> from climada_petals.hazard import WildFire
> import os
> d_path = os.path.join(DEMO_DIR, "Portugal_firms_June_2017.csv")
> firms = pd.read_csv(d_path) 
> wf_pt = WildFire()
> # Note the variable re-assignment here
> wf_pt = wf_pt.from_hist_fire_FIRMS(firms, centr_res_factor=1./2.5)
> wf_pt.plot_intensity(event=0)

[peanutfun]

@riley-brady Thank you for the thorough report. Indeed, the usage of the new methods changed from the deprecated ones and the docs are not completely clear about that. We moved away from instantiating "empty" objects and instead use classmethods now, which return a properly instantiated object.

I agree that we should update the tutorials and use the new methods exclusively. Would you be willing to update the tutorial and provide a pull request?

[riley-brady]

Unfortunately I do not have the capacity right now to work through editing the full tutorial. Work and personal life are quite busy currently. Just wanted to point out this issue, since folks on my team were running into a wall with the current documentation. I'll be looking at this again in the coming weeks and will open a PR if I do end up with some time for this. Thanks!

[peanutfun]

@riley-brady Thanks for reporting back. We assigned this a low priority on our internal to-do list, and we would gladly accept a contribution by you or your team at a later time.