wtchg-kwiatkowski / observatory-web

0 stars 0 forks source link

Genome Browser: Chrome becomes unresponsive and errors when the first "Position" column is chosen as a "Colour By" option in the "Variants" channel settings #337

Closed leehart closed 5 years ago

leehart commented 5 years ago

Home > View genome > Variants channel > open settings > Colour By: Position [the first one]

I've got a feeling this might be a duplicate...? (Or at least related to another issue...?)

invariant.js:42 Uncaught Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
benjeffery commented 5 years ago

What cardinality is reasonable for exclusion from the list? Do you think users would understand why a column is missing?

benjeffery commented 5 years ago

Guess we could grey out with a message. Or the legend component could detect and prevent rendering itself. Thinking about it if the later option solves the issue then I would prefer it, as it can make sense to choose globally high cardinality columns of their local cardinality in the region of interest is small.

leehart commented 5 years ago

I don't think it makes sense to be able to colour by position (or chromosome) here, so I don't expect the user would be surprised (or complain) that such options were missing.

I wonder if there are some existing graph-colouring theories that put an empirical ceiling on the number of colours a human can reason about.

I'd prefer missing to greyed out, for the sake of simplifying the interface and avoiding the inevitable "Why is this greyed-out?" question (compare to "Why is this option missing?"), but either would solve this issue.

I think I see what you mean about it depending, for some options....

leehart commented 5 years ago

I guess self-detecting would avoid the maintenance issue of deciding which columns make sense, but if the legend component simply didn't render itself, with no explanation, then I expect that would be bad user-experience, especially compared to simply not having an option that could cause something bad / useless.

benjeffery commented 5 years ago

It would show the first few plus a message about there being thousands of others.This isnif the fetching is tractable-needs testing

leehart commented 5 years ago

I see. As a first estimation, I reckon not showing more than 20 colours seems reasonable...?

leehart commented 5 years ago

But I still think, ideally, options like "Chromosome" shouldn't even appear in the list. It makes no sense. It's utter clutter! Why would someone want to colour the variants by chromosome, for example, when they're all on the same chromosome?!

benjeffery commented 5 years ago

Yes, you wouldn't want to, agree it makes no sense. So it comes down to what sort of tool this is. Excel/origin/etc would give you the option. I don't think it is worth the manual overhead as the configuration becomes O(num_cols * places you can select them). But I can't think of an automatic way of deciding if a column makes sense for this control.

leehart commented 5 years ago

I can't think of the perfect algorithm for deciding that either, but I reckon that counting the colour-options seems like a reasonable proxy. Maybe the limit could be another config option...!? Are there cases for showing more than, say, 20 colours (in observatory-web)?

Personally I'm trying to treat the demands of the observatory-web implementation differently to the more general powers of the Panoptes platform, otherwise it seems like putting the cart before the horse, if the scope and nature of Panoptes drives the powers and limitations of its implementations.

I can imagine the config could look something like this (yeah, it does look a bit ugly): pf_variants/settings

properties:
- id: chrom
  allowAsColourPropertyInPerRowIndicatorChannel: no
  allowAsColourPropertyInPerRowNumericalChannel: false

I expect the default behaviour of showing everything would avoid config-hell, but I know what you mean.

leehart commented 5 years ago

Is this no longer reproducible on staging? Home > Data > P.f.6.0 Release > Genome Open the config area (cog icon) for the first Variants channel. Colour By: Position Open the info area (i icon) for the same Variants channel. See that Position is given as a range, i.e. 19 to 3255707. The original issue log records that there used to be 2 different Position options (an issue in itself), which seems to have been resolved, so perhaps this one is resolved too (for the observatory-web case... but might cause potential future Panoptes issues.)

leehart commented 5 years ago

This might be the same/similar or different. Home > Data > P.f.6.0 Release > Genome Open the config area (cog icon) for the first Variants channel. Zoom in to see variant markers (triangles). Colour By: Change See error.

leehart commented 5 years ago

I reckon the old duplicate Position column is now named Location. I haven't been able to reproduce the same error (as this issue) with the Location option yet. I suspect the other error (via Colour By: Change) is a different kettle of nettles.

leehart commented 5 years ago

I want to close this issue, and possibly re-log it under Panoptes core, but I have no explanation yet why the problem went away; I can't reproduce the error on staging, and it even generates an insane number of colours (one for each location on the genome), without any errors or performance issue. It's great, but I need closure! :-)

leehart commented 5 years ago

Closing because I can Colour By Location on every chromosome (except Pf_M76611) on staging without any problems, apparently. Impressive! :-) (...How!?! ...Why!?!)