issues with `_check_dataset_form`

[mathause]

The current version of _check_dataset_form has some issues

mesmer/mesmer/_core/utils.py

Line 141 in 57b6b14

def _check_dataset_form(

• requires_other_vars: bool is not granular enough.

  • there is no way to disallow additional variables
  • it's unclear if the additional variables can have any name or must have an explicit one

  I thought replacing requires_other_vars with:

  other_vars : "allowed" | "forbidden" | "explicit" | "required", default: "allowed"
      Behavior when additional variables are on the Dataset
        
      - "allowed": any number of additional variables may be on the Dataset
      - "forbidden": only variables listed in `required_vars` are allowed
      - "explicit": potential additional variables must be listed in `optional_vars`
      - "required": additional unspecified variables are required
  

  is that too much?

• I am not so sure if the logic to check for additional variables is correct & the use of optional_vars seems strange

  • there is currently the following test:

    mesmer/tests/unit/test_utils.py

    Line 319 in 57b6b14

    ds = xr.Dataset(data_vars={"var": ("x", [0])})

    mesmer/tests/unit/test_utils.py

    Lines 326 to 329 in 57b6b14

    	with pytest.raises(ValueError, match="Expected additional variables on obj"):
    	mesmer._core.utils._check_dataset_form(
    	ds, optional_vars="var", requires_other_vars=True
    	)

    
    why does this error? There is no required_var and there is an optional_vars named var -> so I think it should not error

[veni-vidi-vici-dormivi]

Hm I agree that's weird.

Do we have a use case for required in our tests? Otherwise I find that one superfluous.

[mathause]

Maybe the linear regression (I think that is what I originally wrote the check for).

        _check_dataset_form(
            params,
            "params",
            required_vars={"intercept", "fit_intercept"},
            optional_vars="weights",
            requires_other_vars=True,
        )

So there should be additional variables beyond the intercept and the optional weights...

[veni-vidi-vici-dormivi]

Ah requires_other_vars besides optional_vars. Hmm.... Yeah okay.

[mathause]

Well yes its not very clear :-|