[Bug] Set params

[samihamdan]

When the user provided parameter_grid is only including one choice per parameter, the _prepare_hyperparams function in prepare does not work.

[fraimondo]

Wait, it works. By looking at your commit the problem is when they specify a list of one item.

[samihamdan]

for param, val in hyperparams.items():
        # If we have more than 1 value, we will tune it. If not, it will
        # be set in the model.
        if hasattr(val, '__iter__') and not isinstance(val, str):
            if len(val) > 1:
                to_tune[param] = val
            else:
                logger.info(f'Setting hyperparameter {val}')
                pipeline.set_param(val)  # Here
        else:
            pipeline.set_params(**{param: val})
    return to_tune

Where I wrote the comment here is a bug from what I see.
There is no set_param from what I know.
This happens as you mentioned when there is a list of one entry.
I assume when users provide a list with one entry they want to set the param to the one entry.
Or should it be set to the list of one entry?

Anyways it will not work, but the bugfix would be different.

[fraimondo]

We actually never tested that line!

https://codecov.io/gh/juaml/julearn/src/main/julearn/prepare.py

I considered the case when I did the code, that’s why if it is an iterable (but not a string) it checks that it has more than one element to tune and if it is one element, it sets.

Tricky question. What if the underlying transformer requires an iterable?

So your solution of indexing [0], makes this the behaviour:

param_grid  = {'a_param': [a_val]} 

sets a_param to a_val,

while something like this:

param_grid  = {'a_param': [[a_val]]} 

sets a_param to [a_val].

The current solution (not indexing), makes this the behaviour:

param_grid  = {'a_param': [a_val]} 

sets a_param to [a_val],

However, if the underling parameter can be an iterable with several elements, the second solution does not allow this to happen.

param_grid  = {'a_param': [a_val1, a_val2, a_val3]} 

will tune a_param

and

param_grid  = {'a_param': [[a_val1, a_val2, a_val3]]} 

will set a_param to [[a_val1, a_val2, a_val3]] (list of lists).

So I guess we need to index [0].

[samihamdan]

We fixed this Bug in #97, so I will close this.