Add optional arguments to functions besides model_ds about nlmod HOT 5 CLOSED

I have a suggestion for an alternative approach, which might even co-exist and might not be mutual excusive.

What about that every function that makes use of model_ds starts with a list that contains which data_vars and coords it uses from model_ds. More or less similar to what __slots__ is to modules, which announces which methods are available from the module/class. Right after defining this list, we call a checking function that has model_ds and the list as arguments. That function checks if all vars are already part of model_ds or, if not, provides hints for which functions/example notebooks can be used to generate those vars. I see two advantages over your suggestion:

• We can centralize the hints in a single function.
• I think we should stick close to the model_ds for generating the packages, and put as much in model_ds as is needed to generate those packages. Your approach allows for flexibility, but is that flexibility needed/desired?

The parameter descriptions you just gave, are great to put in the attrs of each corresponding data_vars, and of cours in the hints of the checking function.

What is your take on this?

from nlmod.

I'm not entirely sure, but isn't there a way to document it slightly different?

def ghb_from_model_ds(model_ds, gwf, da_name):
    """get general head boundary from model dataset.

    Parameters
    ----------
    model_ds : xarray.Dataset
        dataset with model data.
    gwf : flopy ModflowGwf
        groundwaterflow object.
    da_name : str
        name of the ghb files in the model dataset.

    Model_ds data_vars
    -----------
    ghb_cond : xarray.DataArray or None, optional
        dataarray with the conductivity of the ghb. All non-zero cells are used
        as ghb cells. If None the function argument model_ds should be defined.
        Default is None
    ghb_stage : xarray.DataArray or None, optional
        dataarray with the stage of the ghb. If None the function argument
        model_ds should be defined. Default is None
    gridtype : str or None, optional
        can be 'structured' or 'vertex'. If None the function argument model_ds
        should be defined. Default is None
    only_first_active_layer : bool, optional
        if True only the ghb is only added to the first active layer in the
        model. Default is True
    first_active_layer : xarray.DataArray or None, optional
        dataarray with for each cell the first active layer. Only necesarry
        when use_first_active_layer is True. Default is None
    only_active_cells : bool, optional
        if True ghb cells are only to cells with an idomain of 1. Default is
        False.
    idomain : xarray.DataArray or None, optional
        dataarray with the idomain. Only necesarry
        when only_active_cells is True. Default is None

    Returns
    -------
    ....

Would something like that be allowed in the documentation standard we're using?

from nlmod.

I like your solution a lot and I think this the way to go.

When I tried to implement my original plan I didn't like all the function arguments with the default None value. I think it makes the function less readable.

from nlmod.

Use DataArray source attribute to indicate source of data and maybe log (DEBUG level) when writing MF6 packages where data is coming from.

Linked to #48

from nlmod.

Solved by PR #188

from nlmod.