Filter map anns number 10m m

[will-moore]

Bugs noticed when preparing for workshop...

When filtering by Key-Values, we test each value for being a number.
But previously, values such as 10mM weren't considered numbers.
Now, we allow numbers to have 'units' (any suffix after the number) BUT ensure
that ALL the values have the same units, since we can't convert between units (see discussion below).
If various different units are found, or any value isn't a number then we revert to 'string' field.

To test:

• Try when 1 or more values are not numbers or have different units. This should revert to normal string matching (with drop-down auto-complete menu). First screenshot.
• Try filtering when all values ARE numbers with no units or ALL the same units, e.g. 10mM, 20mM, etc 2nd screenshot.
• If number filter, you can only enter a number (not units)

[jburel]

converting to number leads to some unexpected filtering results (user-3).

[will-moore]

Sorry, I'm not doing units conversion in the filtering!
If that's a blocker then we just have to ignore anything that's not strictly a number.

[jburel]

you indicate in the description that the filtering broke.
This broke the filtering. Since we are trying to fix the issue it is better to do it properly

[will-moore]

There was a bug before because I was trying to convert strings to floats when the Map-Annotations loaded but if I found a value that wasn't strictly a number (e.g. "10mM") then I stopped trying to convert to numbers, so I was left with a mixture of strings and numbers.
That is fixed, so you can now filter for value > 5 and you will find 10mM images.

But to try and understand the units is way beyond the scope of this fix.
e.g. 3mins wouldn't be recognised as units in OMERO. So you'd need some error reporting to say 'invalid units' and to suggest '3 minutes' instead. Also "10mM" wouldn't be understood since we only support time and length units.

One possibility is to enforce that ALL "units" are the same. e.g. you would only show a "number" filtering if all the numbers have the same string suffix. E.g. 0mM, 1mM 10mM, but if you also got "1M" or even "1" then we would bail out and say "you have to use string filtering".
We'd need the same enforcement on the filter type input, to only allow numbers to be entered (and to maybe even show the 'units' of "mM")?

[will-moore]

Description updated to new "units" behaviour.

[jburel]

I can't know add unit as described in the PR
One issue I noticed, depending on the key/value I cannot remove the filter i.e. not cross. see screenshot

[will-moore]

Thanks - tried to reproduce on merge-ci...
But I get X for all filters:

[jburel]

Got the problem
You are looking at a monitor so the browser window will be much bigger and in that case it will be displayed.
I was using the laptop screen only and I was using the default size of the window i.e. I did not resize the window that I just opened and in that case the x is not displayed. If I move the right split pane handle then it shows up

[will-moore]

Ah, OK. I wasn't testing with a monitor, but you're right about the screen size.
Just pushed a fix that prevents wrapping of the X at that size:

[pwalczysko]

One possibility is to enforce that ALL "units" are the same. e.g. you would only show a "number" filtering if all the numbers have the same string suffix. E.g. 0mM, 1mM 10mM, but if you also got "1M" or even "1" then we would bail out and say "you have to use string filtering".
We'd need the same enforcement on the filter type input, to only allow numbers to be entered (and to maybe even show the 'units' of "mM")?

For this PR
Enforcement on the filter type input - hum, but the KVPs do not have to come from the client. Also, such enforcement would be perceived as "silly" - it is perfectly valid to have a wide range of units in your experiment, especially for concentration rows etc.
I would suggest to let the user input units or not as they see fit. But, when filtering, give me a warning that you cannot filter by numbers because the units are not the same - I will as a user recognize the problem and adjust my inputs next time if I really want to filter like this.

Wider discussion:
Btw, this is a wider OMERO problem I think - are then all our annotations just strings (in end effect) (and ROIs) ?

Maybe for another PR:
https://docs.openmicroscopy.org/omero/5.6.0/developers/Model/Units.html shows quite a range of units which we support - maybe we could teach the client to support them properly on input ?

[will-moore]

@pwalczysko Not sure what you mean by "KVPs do not have to come from the client"?
The amount of work it would be to support various units is way too much for this use-case.
Remember the amount of work it was to add units support to OMERO? And that was just for lengths and time. We have no support for concentration, temperature etc. And we don't allow users to pick units or enforce units when entering KVPs e.g. if some uses secs, seconds, s, mins, hrs etc. how to we handle all that?

Originally this PR was simply trying to fix the bug we saw when user entered values of e.g. 10, 15, 10 mM, where the filtering data structure would contain a mix of numbers and strings, causing unpredictable filtering behaviour.
So, now we make sure that you enter the values consistently, otherwise we revert to string filtering behaviour. I think that is an improvement and it means that we don't accidentally compare different units (mins to hrs) etc as in JM's example.
So, we don't give a false sense that we understand the values as numbers, when we don't.

[pwalczysko]

"KVPs do not have to come from the client"

MapAnnotations can be added as a result of a run of a script. Maybe I did not understand your remark about enforcement at the filter type input - I took it to be enforcement at the KVP input, sorry.

[will-moore]

Ah, yes "We'd need the same enforcement on the filter type input, to only allow numbers to be entered" means that when you are filtering by number you can only enter numbers, not "3mins" as JM did above.

[pwalczysko]

And we don't allow users to pick units or enforce units when entering KVPs ...

Well, precisely. Of course this will not be done in this PR. But users want precisely just that. In practically every workshop - "Give me the means to police/unify the user inputs" - clear message form all.

[pwalczysko]

I think there is still a bug.

I have a dataset https://merge-ci.openmicroscopy.org/web/webclient/?show=dataset-14769 user-3
There are PBS keys with values "10mM" each, except one, whose value is "10M".
If I filter for "10mM" all is good - and I get a nice suggestion too, see below.
But if I filter for "10M", also with nice suggestion, I do not get one resulting image, instead, I think I am getting no filtering at all - definitely the "10mM" ones are in the central pane too.

[will-moore]

@pwalczysko String comparisons are case insensitive. That hasn't changed in this PR. (https://github.com/ome/omero-web/pull/147/files#diff-d1ac2bf5c2ac41a675b7576648fb21b8R163).
I could change that, but e.g. if you were looking for "Metaphase" and didn't find "metaphase", would you mind?

[pwalczysko]

@pwalczysko String comparisons are case insensitive. That hasn't changed in this PR. (https://github.com/ome/omero-web/pull/147/files#diff-d1ac2bf5c2ac41a675b7576648fb21b8R163).
I could change that, but e.g. if you were looking for "Metaphase" and didn't find "metaphase", would you mind?

Yes, I might mind. Again, it shows that the feature is hard to understand I did not of course know that string comparison is case insensitive. And the suggestion in the dropdown is plainly misleading - you are suggesting two cases in the dropdown (10M and 10mM , then actually do not treat them differently when one is selected ? That is very unexpected. I would guess for now, change the behaviour to be case sensitive, and maybe, give me a "case sensitive" checkbox when I start to filter ?

[pwalczysko]

And the suggestion in the dropdown is plainly misleading - you are suggesting two cases in the dropdown (10M and 10mM , then actually do not treat them differently when one is selected ?

Actually, this is a bigger problem. I can see now that if I have two terms, such as "concentration" and "concentrationA", then there is no way to filter one out - I will always get images with both "concentration" and "concentrationA"- I will always get both, right ? This is different from our search behaviour, and is not explained anywhere.

[will-moore]

If you filter for "concentrationA" then you won't see images with "concentration".

All filtering for strings (e.g. filter by name or for kvp) allows you to type part of the string.

We have the same filtering behaviour in Parade. E.g. "When the Map Annotations are loaded, pick the Key Gene Symbol and enter the Value CEP to show all CEP genes and then CEP120 to show only images with that gene." (https://omero-guides.readthedocs.io/en/latest/parade/docs/omero_parade.html)

[pwalczysko]

If you filter for "concentrationA" then you won't see images with "concentration".

Aha, yes, so it is not too bad.
I would still go for the case sensitivity - actually, could it be that the term in the box is case sensitive, but the suggestions in the drop-down would not be case sensitive ?
This would mean that if I type "10M" I would get as a suggestion both "10mM" and "10M". But if I select "10M", then this is already case sensitive once it comes into the box -> I will not see any "10mM" images ?

[will-moore]

OK, so the filtering is now case-sensitive and the auto-complete was already NOT case sensitive.
So, you will see auto-complete matches even when all the images have been filtered out:

But when you choose a drop-down option, it will filter as expected.

[jburel]

Tested the above: filter is now case sensitive.

[pwalczysko]

This works nicely now. #147 (comment)

[pwalczysko]

Still an outstanding mystery on https://merge-ci.openmicroscopy.org/web/webclient/?show=dataset-14769 of user-3.

On the images of that dataset, I have for key temperature a mixture of Number+nothing and Number+space+C . (C = Celsius), when filtering for temperature reverts to strings.
Whereas when I have a mixture of Number+nothing and Number+space+mins for key time then the filtering is still number-like

@will-moore any idea why this difference ? Actually, the more surprising is the temperature, I would expect the filtering to be number-like.

[will-moore]

Fixed: Now, if some numbers are missing units and some have units, as long as all the units are the same, then it should be handled as a number field.

[pwalczysko]

@will-moore thanks, works fine now. There was a small apostrophy on one of the temperature values next to the C - I removed it, and now all is as expected.

Ready to merge fmpov