search

statgen / pheweb

A tool to build a website to browse hundreds or thousands of GWAS.
MIT License
154 stars 65 forks source link

update autocomplete to be more comprehensive #166

Closed hanayik closed 5 months ago

hanayik commented 3 years ago

It does not break early, rather it now extends the result list

pjvandehaar commented 3 years ago

I initially limited the autocomplete to 10 results because I think that scrolling dropdowns confuse users. But when I search for a common string like “cancer” or “diabetes” it would be nice to see more of the results. 👍

Two comments then:

  1. I think https://github.com/statgen/pheweb/blob/29878fc7e1e467ef76efbbd3c728670c7bfa5155/pheweb/serve/static/common.js#L46 will show only 100 results, so either raise that to 1000 or lower this to 100.

  2. Is it slow? On a big dataset will it take >100ms? If you haven’t checked this I can try. I’m mostly worried about rsid autocomplete. Fast autocomplete is important to me, and perhaps 1000 is too much on slow internet. We’ll have to test it.

hanayik commented 3 years ago

Yeah, if the drop down has many items I can see how that could lead to confusion. My intent here is that a good enough search term may help prevent the dropdown from ever actually showing 1000 items. But I guess the chance is not zero that 1000 items might be returned. I'm tempted to raise https://github.com/statgen/pheweb/blob/29878fc7e1e467ef76efbbd3c728670c7bfa5155/pheweb/serve/static/common.js#L46 to 1000 (or some other large number) since 100 may still not show all relevant results on occasion. What do you think?

I have not experienced a huge slow down. This edit is live here: https://open.win.ox.ac.uk/ukbiobank/big40/pheweb33k/. You can try searching for "thal" (short for thalamus).This is actually the use case that I aimed to fix. A contributor to this data mentioned that the search for "thal" was not listing results they knew where available. So I changed the logic for returning the autocomplete results, and bumped up the number of returned results.

Let me know if you experience any slow downs if you browse the big40 URL above.

If you're happy with increasing the autocomplete limit in the JS to 1000 then I can make that change and add it to this PR.

Cheers,

Taylor Hanayik

On Thu, Jul 8, 2021 at 3:58 PM Peter VandeHaar @.***> wrote:

I initially limited the autocomplete to 10 results because I think that scrolling dropdowns confuse users. But when I search for a common string like “cancer” or “diabetes” it would be nice to see more of the results. 👍

Two comments then:

1.

I think https://github.com/statgen/pheweb/blob/29878fc7e1e467ef76efbbd3c728670c7bfa5155/pheweb/serve/static/common.js#L46 will show only 100 results, so either raise that 100 or lower this 1000. 2.

Is it slow? On a big dataset will it take >100ms? If you haven’t checked this I can try. I’m mostly worried about rsid autocomplete.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/statgen/pheweb/pull/166#issuecomment-876510856, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHZ3OE666BBMA3DRPPMTT3TWW4JBANCNFSM5AAOO4JQ .

jhchung commented 2 years ago

Hi @pjvandehaar,

I had the idea to do the same change for search and saw this pull request from @hanayik.

My changes are very similar to Taylor's, but I added some options to conf.py to allow the user to changing the total number of results and whether to extend or break out of autocomplete.

Not sure how to go about making a pull request because we touched the same bit of code.

Best, -Jonathan