larray-project / larray-editor

Graphical User Interface for LArray
GNU General Public License v3.0
2 stars 2 forks source link

add support for dicts in compare #230

Open gdementen opened 2 years ago

gdementen commented 2 years ago

I just had to compare many sessions and naturally put them in a dict of sessions, then wanted to compare them and tried compare(dict_of_sessions) but it does not work (compare(dict_of_arrays) does not work either) but the workaround felt very clunky:

compare(*dict_of_arrays.values(), names=list(dict_of_arrays.keys()))
compare(*dict_of_sessions.values(), names=list(dict_of_sessions.keys()))

On the other hand, I know that forever expanding the API is not a good idea, but I see this akin to what we support for rename, set_labels etc.

Maybe we should deprecate the names= argument of compare (and possibly even the *args syntax) and instead replace it with:

compare(obj1, obj2=None)

which we could thus call either:

compare(obj1, obj2)
compare(dict)
gdementen commented 2 years ago

I stumbled on this again and see really no valid reason not to support this. I am unsure about deprecating the names argument and the list/tuple of objects syntax. I think it would be better but is it worth "breaking" (at first it would just be a warning) backward compatibility? I don't know. Anyway, this could come later.

alixdamman commented 2 years ago

I also think we should start with a warning (and an issue to not forget to deprecate the names argument in the (short) future).

gdementen commented 1 year ago

Deprecating the *args and names argument seems like a bad idea because it makes it impossible to compare arrays with the same name (see #247).