param icon indicating copy to clipboard operation
param copied to clipboard

Fix all_equal() and hence get_param_values(onlychanged=True)

Open ceball opened this issue 5 years ago • 0 comments

all_equal() is pretty broken, so replace it with Comparator.is_equal().

Fixes #179 Incorporates #180

But, some issues...

To do:

  • [ ] decide on "different definitions of equality"
  • [ ] decide on how/when to suppress name parameter from get_param_values()
  • [ ] add a test (if necessary, depending what is decided)

Different definitions of equality

I'd like everything to be deciding equality the same way. However, it's probably tricky. You can't always have full equality checks everywhere (e.g. could be too expensive).

Comparator.is_equal() does not (by default) handle numpy arrays or param.Time(*), both of which are (meant to be) handled by all_equal().

  1. Drop those from all_equal() (i.e. make it like default of Comparator.is_equal())?

  2. Or support those during all_equal(), i.e. retain all_equal's existing intended behavior, but comparisons done with Comparator (outside of all_equal) would by default behave differently.

  3. Or make Comparator.is_equal() handle numpy arrays and param.Time by default. (I'm not quite sure how it's supposed to work on skimming it, but it looks like Comparator is designed to allow comparisons without explicit imports, but even if not seems likely it could be made to work).

(*) Has _infinitely_iterable attribute

I'm not too sure. I prefer 1 or 3. When I think of param in the past, I think 3 is what I'd like. But I suspect there's a good reason the default of Comparator is not to handle various cases (performance?).

Retaining get_param_values(onlychanged=True) behavior of not showing auto-generated name

As noted in #179, a side effect of the incorrect all_equal() function is that it suppresses a parameterized's name from appearing in the result of get_param_values(onlychanged=True) if the name was auto-generated. I've added suppress_auto_name to get_param_values(), defaulting to True, so that if onlychanged=True is requested, an auto-generated name is not included.

But I feel like there are often times when you don't want the name in the parameters you're dealing with, so I'd prefer suppress_auto_name to be respected whether or not onlychanged is True.

I haven't thought about it a lot. Whether to have this option and what it does might depend a bit on the outcome of the api discussions.

ceball avatar Sep 14 '20 20:09 ceball