ClinGen / clincoded

This GCI/VCI 1.0 platform has now been retired, and replaced with our new 2.0 platform:
https://github.com/ClinGen/gene-and-variant-curation-tools/issues
MIT License
25 stars 9 forks source link

Experimental Data Page (was Functional Data Page) #89

Closed hitz closed 8 years ago

hitz commented 9 years ago

Need a page per specs for viewing and editing functional data as it applies to a GDM.

Note that the functional data is associated with the gene in a disease context, BUT we will need to query "all" functional data for a given gene (with disease factored out)

selinad commented 9 years ago

I'm not sure disease will be able to be 'factored out,' if I am understanding this expression correctly - we will need to find all information associated with a gene upon a gene query, including functional info, but it will be need to be clear in which GDM(s) the gene information exists upon query. It's possible a piece of functional evidence could be used for the same gene associated with a different disease if the curator deems it truly independent of the first disease (perhaps this is what you mean by 'factored out')...the query should just make clear any existing GDM associations.

hitz commented 9 years ago

this depends on functional sample data from #21

selinad commented 9 years ago

At last week's small gene curation meeting it was decided that Functional Data will now be called Experimental Data, as an FYI for anything that needs to be updated.

mrmin123 commented 9 years ago

@selinad prelim version of experimental data curation page has been incorporated here: http://89-mc-experimental-data-page-70019d0-minchoi.instance.clinicalgenome.org/dashboard/ (disabled as of 2015-09-18)

Added Experimental Data items can be seen here: http://89-mc-experimental-data-page-70019d0-minchoi.instance.clinicalgenome.org/curation-central/?gdm=a670526d-1b2b-41d6-bd55-2e0bca960171&pmid=123456

There are some bugs/improvements that I haven't managed to get in yet:

Some possible tests

  1. Create GDM/Navigate to GDM
  2. Select evidence
  3. Add an Experimental Data type
  4. Try viewing it
  5. Try editing it
  6. Repeat steps 3-5 for the 5 other different Experimental Data types

Corrections on the field names and messages would be much appreciated!

selinad commented 9 years ago

Hi @mrmin123 - Here is some feedback based on preliminary testing. Again, a lot of great work! I will edit this comment tomorrow as I have more things to add....

Here are the abbreviations I am using: BF = Biochemical Function INT = Interactions EXP = Expression MS = model systems R = Rescue FA = functional alteration

Note: Variant entry field needs to be added where specified on pptx - do this based on variant info on Family page

Functionality changes

Text changes

Bugs

The server erred while rendering the page.

TypeError: Cannot call method 'toString' of undefined
    at [object Object].React.createClass.render (/srv/clincoded/src/clincoded/static/components/experimental_curation.js:1579:83)
    at [object Object].ReactCompositeComponentMixin._renderValidatedComponentWithoutOwnerOrContext (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactCompositeComponent.js:789:34)
    at [object Object].ReactCompositeComponentMixin._renderValidatedComponent (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactCompositeComponent.js:816:14)
    at [object Object].wrapper [as _renderValidatedComponent] (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactPerf.js:70:21)
    at [object Object].ReactCompositeComponentMixin.mountComponent (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactCompositeComponent.js:237:30)
    at [object Object].wrapper [as mountComponent] (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactPerf.js:70:21)
    at Object.ReactReconciler.mountComponent (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactReconciler.js:38:35)
    at ReactDOMComponent.ReactMultiChild.Mixin.mountChildren (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactMultiChild.js:192:44)
    at ReactDOMComponent.Mixin._createContentMarkup (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactDOMComponent.js:289:32)
    at ReactDOMComponent.Mixin.mountComponent (/srv/clincoded/src/clincoded/node_modules/react/lib/ReactDOMComponent.js:199:12)
mrmin123 commented 9 years ago

@mrmin123 - tried to answer your :question: with a :speech_balloon: (so proud) - hope they make sense

@selinad : http://89-mc-experimental-data-page-175838c-minchoi.instance.clinicalgenome.org/ (live as of 2015-09-16) (disabled as of 2015-09-18)

Test GDM: http://89-mc-experimental-data-page-175838c-minchoi.instance.clinicalgenome.org/curation-central/?gdm=72818d1a-b357-4b70-b919-3287ac14a9d5&pmid=123456

Everything with a green checkmark above has been fixed/addressed as of 175838c (above instance)

Things still not yet addressed are as follows (taken from both @selinad's and my previous post) - my comments are in italics after the :question: symbol:

Functionality

New Item(s)

mrmin123 commented 9 years ago

Updated instance (as of 2015-09-18): http://89-mc-experimental-data-page-9412ba1-minchoi.instance.clinicalgenome.org/ (down as of 2015-09-24)

Previously instances have been taken down.

selinad commented 9 years ago

Thanks, Min - I will get to this this afternoon.

selinad commented 9 years ago

Super job checking off all of those items, @mrmin123!

I am going to comment by evidence_type:

BIOCHEMICAL FUNCTION

Biochemical Function (and Expression - A and B types)

selinad commented 9 years ago

PROTEIN INTERACTIONS

selinad commented 9 years ago

EXPRESSION

selinad commented 9 years ago

FUNCTIONAL ALTERATION

selinad commented 9 years ago

MODEL SYSTEMS

selinad commented 9 years ago

RESCUE

mrmin123 commented 9 years ago

@selinad, this is regarding this item:

image I chose blue because that's the 'info' color, but it's an easy change to yellow/green. Also, should I move the info box to immediately after the Experiment type dropdown? The potential issue with that would be that the location of the Experiment name field would move up and down depending on what is chosen in the previous dropdown. Am I making sense?

selinad commented 9 years ago

Looks good, @mrmin123 ! I defer to you and @forresttanaka as to whatever color makes sense wrt the interface - I like the blue, though, and it does seem to be most consistent with where blue is in the resto f the interface.

I think the box is OK as is...it will always be close to the type since what is between it and the type is always the same (the name box) - thank you for checking, though. Looks nice.

mrmin123 commented 8 years ago

@selinad Regarding the header, how does this look?: image

Other notes:

Biochemical Function A: Asterisk only appears for "Evidence that gene(s) share same function once gene symbol entered above....seems strange

This is because if the field is always set to required, even if the user wants to submit information only for section B, the form will error out because they will be expected to enter 'Evidence that gene(s) share same function', even if they haven't specified a gene. I could disable requirement for the field entirely, if that's what you prefer.

All types: For Variant, they can enter only one or other - VariationID or Other Description - so, typing in one should prevent typing in other

We briefly talked about this and I noted this above, but I can't manage to reproduce this, but I'll keep an eye out for it. If you manage to reproduce this in a future instance perhaps you can recount the steps you did (may involve form erroring and/or changing of experiment Type??) or show me?

selinad commented 8 years ago

Hi @mrmin123 - yes the title looks great!

Thanks for the explanation - the field needs to stay as required, so no problem.

I'll let you know if I reproduce the problem - if it is working right for you, that's great!

mrmin123 commented 8 years ago

Model Systems: Need to check on cell-culture tpye/line and whether it is controlled vocab. Made note to self. Right now, it is all caps. BUG - enter text into Cell-culture tpye/line and get no submit when I hit save (happens in new or edited entry).Is there a required data type here?

The schema says it should be EFO ID, so I'll set it to that... the submit not working is a separate bug I'll have to resolve.

mrmin123 commented 8 years ago

@selinad Working on the new behavior for the blue boxes: image

It looks kind of weird to me with the information in the blue box being almost the same as the text in the dropdown right below it. Is this what you had in mind?

selinad commented 8 years ago

Thanks, Min - I think it's OK - it just verifies they've selected the correct item and it gives them an easy visual to remember what they are curating towards.

This is different for A, right, where the description for A is longer than the pull down wording. The other option would be to put the select A or B pull down above the blue boxes - then the correct one shows after selecting from the pull-down - would this be more consistent with the other evidence types?

mrmin123 commented 8 years ago

@selinad I think this works. The other evidence types also have the text in the same location:

Biochemical Function: image

Biochemical Function w/ A selected: image

Biochemical Function w/ B selected: image

Protein Interaction: image

Expression: image

Expression w/ A selected: image

Expression w/ B selected: image

Functional Alteration: image

Model Systems: image

Rescue: image

mrmin123 commented 8 years ago

@selinad & @tsneddon: Instance of latest build (as of 2015-09-24): http://89-mc-experimental-data-page-14def68-minchoi.instance.clinicalgenome.org/

Has a bunch of the changes marked above.

Some things that I forgot about/are still missing:

  1. Assessments
  2. Some of the forms shouldn't be submittable when certain criteria aren't met. Specifically:
    • Has this gene or genes been implicated in the above disease? must be checked in Biochemical Function and Protein Interaction
    • Is the gene normally expressed in the above tissue? must be checked in Expression subtype A
    • Is expression altered in patients who have the disease? must be checked in Expression subtype B??? (need confirmation)
    • Does the wild-type rescue the above phenotype? must be checked in Rescue

If I'm mistaken on item 2 above let me know.

selinad commented 8 years ago

Hi @mrmin123 Nice job and thanks - this looks great!

How do you mark which type of Biochemical Function or Expression shows on palette? Maybe: Biochemical Function - A Biochemical Function - B or Biochemical Function (A) Biochemical Function (B)

selinad commented 8 years ago

Thanks @mrmin123

My comments on this instance for the Biochemical Function evidence-types A, B

Features:

Bugs/necessary changes:

mrmin123 commented 8 years ago

@selinad How does this look for the palette?

image

selinad commented 8 years ago

Hi @mrmin123

Thanks! I'm just looking at the palette for Family, etc. The bold is the label (name) for the Family (which the curator provides.

Maybe it should be: BFA1 Biochemical Function (A) Minyoung Choi Date etc.

This has the disadvantage of making it even longer vertically, but it is more consistent. Either that, or we leave the type off, but I think users will want to scan since they are worth different amounts wrt the score

mrmin123 commented 8 years ago

Updated: image

Also, is this what you had in mind for the info box with all the definitions?: image

The box disappears when you select a Experiment type, and reappears if you set it to None

selinad commented 8 years ago

Sweet. I like that better wrt palette.

Yes - thanks - just 2 things:

  1. Add space between the different types so easier to read :white_check_mark:
  2. Put blue box below Experiment type pull-down if easy to rearrange. :white_check_mark:
selinad commented 8 years ago

Comments for Protein Interactions:

Features:

Bugs/necessary changes:

selinad commented 8 years ago

Comments for Expression

Features:

Bugs/necessary changes: EXP-A: again, apologies for the confusion - EXP-A does not need Variant (EXP-B DOES need it) :white_check_mark: EXP-A: change Curation Central link in Note to Record Curation page (as in other notes under checkboxes), + add getting back to experimental :white_check_mark: EXP-B: Need note that follows check-box, with update to get back to new experimental data entry page. Note would say: If the expression is not altered in patients who have the disease, the criteria for counting this experimental evidence has not been met and cannot be submitted. [then sentence I had reworked before for returning to add new experimental or return to record curation page] :white_check_mark:

We should think of adding Associated Variant to the Experimental data info on the palette - let me know if I should push to later version. :white_check_mark:

selinad commented 8 years ago

Comments for Functional Alteration:

Features:

Bugs/Necessary Changes:

selinad commented 8 years ago

Model Systems:

Features:

Bugs/Necessary Changes:

selinad commented 8 years ago

Rescue

Features:

Bugs/Necessary Changes:

mrmin123 commented 8 years ago

@selinad @tsneddon New instance with above changes: http://89-mc-experimental-data-page-c853eea-minchoi.instance.clinicalgenome.org/

I'm thinking of pushing assessments to another ticket/PR to keep the additions and changes somewhat isolated...

mrmin123 commented 8 years ago

Forgot to patch this in the last instance. Variants showing up in the Experimental Data palette:

image

If you'd like a new instance with this change in let me know and I'll spin one up.

selinad commented 8 years ago

Hi @mrmin123 - Wow - incredible work!!! A lot of details to work through - thank you!

I'm not sure how our signals were crossed on this - I'm sorry if it was my fault. Variant needs to be added to the following:

All the other changes you put in look great and I couldn't find any real bugs! I have a few texty things, but I will review with Tam (these will not prevent pull request).

mrmin123 commented 8 years ago

@selinad The thing with Variants is actually a bug in my code! To be sure, however, Variants SHOULD be visible on: Expression (B) Functional Alteration Model Systems Rescue

Is this correct?

selinad commented 8 years ago

@mrmin123

Yes, correct! Ah, good- hopefully it's an easy bug :+1: to fix!

mrmin123 commented 8 years ago

@selinad Yep, fixed now. I'll go ahead and make a PR now if that sounds good to you.

selinad commented 8 years ago

I was happy with it, but if @tsneddon has a chance to run through it and make sure I didn't miss anything functional, that would be great.

tsneddon commented 8 years ago

I just worked through it and didn't see any issues.

selinad commented 8 years ago

Woot! Thanks, @tsneddon; Nicely done, @mrmin123

mrmin123 commented 8 years ago

Great, will put together a PR ASAP. Thanks @selinad and @tsneddon!

kilodalton commented 8 years ago

Great job. Thanks for all your hard work to get the app to production!