search

ome / omero-mapr

An OMERO.web app allowing to browse the data through attributes linked to the image
https://pypi.org/project/omero-mapr/
GNU Affero General Public License v3.0
5 stars 12 forks source link

load case insensitive data to the tree #17

Closed atarkowska closed 7 years ago

atarkowska commented 7 years ago

This PR relaxe queries to load case insensitive data https://trello.com/c/M3q00BhI/55-mapr-case-insensitive-searching

google chromescreensnapz003

Test:

manics commented 7 years ago

Does this need to be an option or could it be simplified so it's always case-insensitive?

atarkowska commented 7 years ago

by default it is case insensitive, the option is to filter with case sensitive, see the example testing links

manics commented 7 years ago

I know, I was wondering whether it could always be case-insensitive to reduce the code complexity.

atarkowska commented 7 years ago

I don't know enough use cases to judge. Up to @eleanorwilliams, we briefly discussed that both options may be needed. This is a breaking change anyway!

eleanorwilliams commented 7 years ago

@manics The casing of the gene symbols is to a certain extent different in different species e.g cdc14 in Drosophila, CDC14 in yeast so if you want to search for the gene in an narrower range of species you can do it using casing.

eleanorwilliams commented 7 years ago

I find it a little disconcerting that the autocomplete list shows the different casing options, but when i select one it gives me back all of them. Maybe we need a check box that says 'return case-insensitive results' to make it more obvious that its going to do this.

The good thing is that in the results it separates the results e.g. cdc14 and CDC14 are listed separately, so that even though the search is case insensitive you can still refine by the the more specific results.

eleanorwilliams commented 7 years ago

of course we also have the issue that the Gene Symbols are not always entered consistently - http://idr-ci.openmicroscopy.org:1880/web/mapr/gene/?value=YFR028C

atarkowska commented 7 years ago

@eleanorwilliams thanks for your suggestion, I will update the UI

atarkowska commented 7 years ago

@eleanorwilliams I am done, ready for review. Please read description for reference of new changes.

eleanorwilliams commented 7 years ago

I like the check box. I personally would not have it greyed out, I'd just have it obvious from the start. And I'd change it to 'Match case'. Otherwise all looks fine.

atarkowska commented 7 years ago

@eleanorwilliams thanks for feedback. updated

atarkowska commented 7 years ago

tests are passing https://idr-ci.openmicroscopy.org:8443/job/MAPR-test/3/console

atarkowska commented 7 years ago

passed again https://idr-ci.openmicroscopy.org:8443/job/MAPR-test/5/console

eleanorwilliams commented 7 years ago

Works fine. I noticed its only for genes and compounds (which are the two with issues) but any reason it couldn't be consistent across all mapr types?

Also the other thing that is slightly annoying is that if you check the box after you start typing the search goes away. E.g. if you type cdc14 in to the gene box, see that there are lots of casing options and then click the checkbox to say you want it to be case sensitive, then the cdc14 that you typed is gone. I don't know if there is an easy way round this.

atarkowska commented 7 years ago

as mentioned in the description availability of checkbox is configurable, it was done on purpose to allow testing and exploring both possibilities.

autocomplete is only triggered on typing, so match cases has to wipe a state of autocomplete otherwise it won't work. We can review that in a future.

pwalczysko commented 7 years ago

The tests https://idr-ci.openmicroscopy.org:8443/job/MAPR-test/ are not accessible to me. Site asks for credentials, and the most obvious username/pwd combinations fail for me.

pwalczysko commented 7 years ago

All works as expected except

atarkowska commented 7 years ago

thanks for testing, please provide final wording for both tooltips :)

pwalczysko commented 7 years ago

Please select "Match case" or not as desired, then start typing to search for Gene symbol or Gene identifier. Select the desired item from the dropdown menu which appears after typing has started.

The second tooltip on "Match case" is either fine or I would remove it completely, it does not really need a reformulation for me, or even be there (maybe @eleanorwilliams will have a better opinion)

eleanorwilliams commented 7 years ago

Text from @pwalczysko is good. I think for the mapr categories where you can't select the match case box the tool tip shouldn't mention it. In those cases just have 'Start typing to search for siRNA identifier or siRNA pool identifier. Select the desired item from the dropdown menu which appears after typing has started.' etc.

eleanorwilliams commented 7 years ago

Tools tips are good now. @pwalczysko what do you think?

pwalczysko commented 7 years ago

Happy with the tooltips too, thank you.

atarkowska commented 7 years ago

@joshmoore is yours now :)

joshmoore commented 7 years ago

Excellent! Demo tomorrow with @jrswedlow at the meeting and then let's get it in. Thanks, all.

joshmoore commented 7 years ago

From @jrswedlow:

Really like the change— so yes, please move into production. I can’t detect any issues with performance, but that might just be me.

atarkowska commented 7 years ago

thx everyone

joshmoore commented 7 years ago

Released as 0.1.11