Closed jimmyzhen closed 6 years ago
@jimmyzhen @gto389 -- would be great to look into OLS API for returning term names for HPO IDs (one of the comments on mockup) -- this would be very valuable for expert reviewers and curators.
Current progress with the summary table.
@selinad, @wrightmw,
As we discussed today, the *Associations** column won't be included in the Case Level table for the time being.
As for the Variant column, bringing in the population data for the given variant won't be trivial due to the following:
As for the Segregation Score column (last column), the segregation points are assigned given the total LOD scores of one or more families. But since each scored proband has only one associated family, and hence only one LOD score is represented. Would it makes sense to assign segregation points to just one LOD score? Or am I misinterpreting the specs?
Thanks, @jimmyzhen -- makes sense.
The spec is for the LOD scores to be summed across the record and then the points assigned, so it would be confusing to assign points to an individual LOD score for a family.
@jimmyzhen Comments on current instance:
HI @jimmyzhen looking fantastic! So much fun to see this take shape. Thanks for all your hard work.
Here are a few things:
How about having "Status" say "Classification status:" (this will be the status of the user's summary, not the record status), and underneath it will say "Date Classification Saved:" e.g.
Classification status: in progress (or Provisional) Date Classification Saved: [timestamp]
Maybe we want to change "Summary" to Classification for the owner, too -- e.g. "Classification Owner"; @wrightmw we should look again at our use of "Summary" and "Classification"
Re scoring -- it definitely seems to be doubling everything -- e.g. doubles the count and the score for all the fields I entered.
For the Experimental -- I put in one A and one B Biochem function evidence, but both say "(Gene(s) with same function implicated in same disease); these 2 should have different descriptions.
Small detail: would be great to have the top table stand out more somehow.
The data I entered is displaying well -- will go back and add some more -- exciting!
@selinad I'm commenting below your comment to keep my response close to your question. Let me know if you find this confusing and I will stop this form of communication. I thought we were going with "Classification Summary" and "Evidence Summary", however I agree that these are fairly similar terms. I think we also need to think about the terms we currently use in the VCI, we don't want to diverge too far from these. However... how about "Classification Matrix" and "Evidence Summary" as an alternative?
Hi @jimmyzhen -- more feedback based on testing
For variant:
For Case-Control:
Still adding more data -- looking good!
@jimmyzhen I think you reminded yourself of this -- need Score status column -- the second one is a "Review" in this table, but not clear:
@jimmyzhen For family segregation info with no proband:
Is it reasonable to add the following columns -- a reference is definitely needed
Maybe change "Case Level (segregation without proband)" to "Case Level (family segregation information without proband data)"
Also, when there is no evidence for this section, it says something about no Experimental evidence, it should say "No segregation evidence for a Family without a proband was found" -- along those lines, we should remove the word "scored" as we will be showing evidence for review and contradicts.
@jimmyzhen for contradicts, "Contradicts" will go in status column and I think we should do n/a in score column? Maybe in red? Or contradicts in red?
@selinad I very much like the idea of putting contradicting data in red.
@jimmyzhen Here is what it looks like for experimental:
@selinad I wonder if we should ask the curators about this? I could see a similar argument for including evidence that hasn't been scored as we do in the VCI, that is 'it's good to see evidence that hasn't been evaluated/scored because you might have missed it and then that gives you the prompt to go back and evaluate/score it'?
I'm also very much in favour of bolding the scores to differentiate them from the default. Or instead, perhaps we could bold both of them when they are different to draw attention to those scores that have been changed?
@jimmyzhen based on discussion we just had, please ad "Age of" before the field (e.g. Report, Onset, etc.) in the table (also in bold). e.g.
Age of onset: 54 years
@jimmyzhen this is looking incredible so far! WOW!
I'm still testing a lot of things -- here's one small thing and we will see if @wrightmw agrees: for the Edit Classification button -- would prefer if it had the word "Edit" in it, even though I like the pencil icon. It took me a while to focus in on the icon and then one wonders what the "Classification" button does. Love the way it anchors to the bottom -- so clean!
Looks great @jimmyzhen !
@jimmyzhen when user hits the edit Classification button, can it also anchor them to bottom? (wait to see if @wrightmw agrees).
Love the Curation Central button with the icon in it! Let's say "Record Curation page" like other places
@jimmyzhen great! Anchor works and button looks great.
@jimmyzhen I'm worried the Previous testing column in case level is coming from the Y/N pull down -- I think it should come from free text below that. @wrightmw do you know?
@jimmyzhen for the proband method of detection, would it be reasonable to put "Method 1:" and "Method 2:" in front of answers? (in bold, like "Onset").
✅ @jimmyzhen looks great!
@jimmyzhen I just confirmed previous testing should come from description not the Y/N --- so sorry for not catching before (or specifying in google doc)
✅ Great @jimmyzhen !!!
Hi @jimmyzhen Here's one with no variant info type, but is marked review:
@wrightmw should we show these? It might be ok, but if we do the "()" looks funny. Not a big deal, but let me know if there is a quick fix @jimmyzhen (i.e. would it be simple to not show if curator does not pick a variant type?)
Looks great @jimmyzhen !
@jimmyzhen per earlier point about previous testing fields: when I select Yes and then add a description, it looks like it might be combining the fields? Kind of funny as there is period at end of "Yes"
✅ The rendering of the "Yes/No" field has been removed.
Great @jimmyzhen ! I only see the description now, not the Y/N field
@jimmyzhen how are you sorting the tables? Can we sort by variant type?
✨ ✅
@jimmyzhen Thanks for the sorting -- fantastic! If quick, one ask would be to have the evidence with no variant type be at the bottom instead of the top (rest of order looks good). Not urgent.
@jimmyzhen -- it used to be the explanation field was grayed out unless they changed from No Selection. Now it's not, but it is still required if there is a modification. I think this is OK (and perhaps that's where we ended up), but can we add an "*" to that field when something other than No Selection picked? thx!
✅
This works now -- thanks, @jimmyzhen !
@jimmyzhen is there a way to make the Evidence Summary window pop open as a new window (not tab) and in a smaller tab (like a modal almost?). Probably not, but thought I would check : )
:man_shrugging: ✅
lol -- I think this means not possible @jimmyzhen !
@jimmyzhen first bug. If I first select Yes to count a LOD score, then Edit to say No, the Matrix seems to update (based on score not counting), but when I go to the Evidence Summary, it still says yes:
I tried doing a shift-refresh and still said Yes
p.s. changed score for a proband and there was no problem
🐛 ✅
@jimmyzhen this bug is fixed -- thank you!
@jimmyzhen just realizing there is no TOTAL score for Case-level evidence...
✨ ✅
@jimmyzhen this is a late ask, but I realize we didn't have the comments field for the whole GDM record when we created the mockup for Summary. Would it be possible to have another "table" below the top one summarizing record that shows the "Evidence Summary" field? It could also have a dark blue header. I think this would be of great value to the reviewers....
✅
This looks fantastic @jimmyzhen !!! Huge plus. thank you!
I am not sure what is wrong wuth this: :P (for both titles).
🐛 ✅
👍 @jimmyzhen !
@jimmyzhen for below:
Could you please change "(Cases)" to "(Case)" under Disease and add "(Case)" under "Detection Method"? thx!
💅 ✅
Looks polished! Thx @jimmyzhen !
@selinad @jimmyzhen If I click "Save" the on the Classification Matrix and then linkout to "Curation Central". Then link back to the Classification Matrix (without making any edits), I now have the option of "Cancel" or "Save" again. I understand it's because the modification and some other fields are editable but the options don't make sense to me... if I've made no changes then shouldn't I have the option of linking straight to the Evidence Summary? Maybe the option to "Save" should only occur if I start to edit any fields? Or maybe when I go back to this page it should all be greyed out and I have to specifically select "Edit" to change anything on the page? Maybe it's just me but it seems strange to have to "Save" again when I haven't made any changes?
Hi @wrightmw: my opinion this is an unusual use case and we shouldn't worry about the logic. I'm not sure I've followed it all though.
@jimmyzhen what field are you pulling from in Bioch fxn B for this empty field (Explanation) -- I filled in all required fields, so it should be one of those, I think:
🐛 ✅
@selinad Responses from above: A - I'm ambivalent on this one. I like the pencil but can also see that "Edit" is more immediately obvious. B - Yes, I think anchoring it would look better C - I don't have a problem with showing them but I agree that maybe a dash or some other identifier to designate a null field would be better than the empty brackets. Although not essential.
@jimmyzhen I forgot to say how wonderful the work you've done is. You've done an incredible amount in a short time. Thanks so much! Much like Father Christmas, I don't believe in elves... this magic is all yours ;-)
🎅 was here
@jimmyzhen if takes 5 minutes, one suggestion I should have thought of earlier: Can you add (A) or (B) as appropriate for Biochem function and Expression to the text below? Not a big deal, only if fast and you are in that section of the code:
e.g. "...disease of interest (A), ...."
💅 ✅
Nice @jimmyzhen !!!
@jimmyzhen another bug I'm afraid. I changed a score to 0 from the default -- it saved correctly in the matrix but shows as the default (0.5) in the summary table:
🐛 ✅
This bug has been squashed, @jimmyzhen ! thank you!
@jimmyzhen I've tried every evidence type and made certain all fields there and everything is great except fields I've noted. Really superb job. Only thing I didn't see was the TOTALS, but maybe OK for now.
@jimmyzhen Amazing -- even the totals are in now! Wow, this is spectacular!
@wrightmw we should probably make sure it works well when we score each other's evidence too. Here's my record if you want to try a few things: https://1411-jz-evidence-summary.demo.clinicalgenome.org/curation-central/?gdm=dad11445-1394-4ad4-a341-5f7e1136160a&pmid=22134555
@selinad I think my explanation is the problem.. here again with screenshots. Imagine I'm a curator who is happy with the all the evidence/scores etc., I don't plan to add/modify anything. I've marked it as Provisional and I'm asking a Reviewer to look at it. Here is my Matrix that I've Saved and marked as Provisional:
Some time later I just want to go back and view everything I did. My only way to access what I Saved is via the View Classification Matrix and when I do that I get this screen:
My only options are "Cancel" and "Save". I have already Saved everything on my previous visit (when I marked it as Provisional) and now I have to "Save" it again, when all I want to do is View the evidence Summary. I feel like I should have the option to view the Evidence Summary without having to re-Save what I've already previously Saved.
Thanks @wrightmw that is easier to follow. It sounds like @jimmyzhen would need to add logic -- he can let you know how straightforward. For now, I think it's OK to ask them to click again as it's (I think) a bit of an outlier case. Maybe this goes along with having the ability to view the evidence summary from the dashboard if no evidence has changed....
thx for the explanation and screenshots. I'll leave it to @jimmyzhen to weigh in!
@selinad Do you think that the rationale as to why a score was increased or decreased compared to the default score should also be included as a column in the table? e.g. a "Reason for change" column.
💅 ✅
Love this! Nice work @jimmyzhen !!!!
Hi @wrightmw I do like that idea! I'm just not sure whether it's in scope for this release, but @jimmyzhen will need to weight in :-)
💅 ✅
is there a way to make the Evidence Summary window pop open as a new window (not tab) and in a smaller tab (like a modal almost?). Probably not, but thought I would check : )
@selinad, we don't have control over the user's browser in this matter.
Hi @jimmyzhen I am adding this based on my discussion with you. Please feel free to ask me to move it to R14 as it is absolutely not necessary for this release. Would be great to display the type of Functional Alteration or Rescue or Model Systems (e.g. patient cells, non-patient cells) either in same column as the evidence type or in a different column (whichever makes most sense).
@jimmyzhen Wow -- you even added this, too -- amazing! Thank you.
@wrightmw,
Are the following comments related? https://github.com/ClinGen/clincoded/issues/1411#issuecomment-334633525 https://github.com/ClinGen/clincoded/issues/1411#issuecomment-334627640
@wrightmw @jimmyzhen Matt, I'm reading your workflow and screenshots with a clearer head. It seems like you are looking for different logic when something is marked Provisional? Does that take us back to the things we were working through the other day? i.e. you would get non-editable classification if it is Provisional? But, we would need to know no evidence added in between, right? It seems like it gets into that other workflow in the Google doc, but not certain if I am understanding.
It's complicated because matrix scoring can be new, Provisional or not. Maybe that's why I was trying to separate out the last saved. Still in drawing mode...
@jimmyzhen Yes, those two comments are related.
@selinad Yes. That's what didn't seem right to me. If you are still "in progress" then I don't think it matters that you are asked to "Save" again. But if you have marked it as "provisional" and then you go back to your Matrix and Summary, then it seems kind of odd that you are being asked to Save it again. I think you should have the option of going directly to the Classification/Summary that you marked as Provisional?
@jimmyzhen I've looked at everything -- only need to check the bug fixes. Absolutely incredible. You added some amazing enhancements, including the HP terms. Thank you so much for your hard work.
Quick questions
Again, fantastic work!!! p.s. comments interspersed in above edits -- not certain whether you receive notifications for these
@jimmyzhen both 🐛 s have been squashed and all your checkboxes work.
I can also confirm the experimental is sorting correctly -- looking great!
Re new workflow additions (thank you for your quick work on this @jimmyzhen !):
I'm wondering if we should move this to right below Total Points and say: "The Total Points shown above is based on the the set of saved evidence and accompanying scores that existed when the "View Classification Matrix" button was clicked. To save a Classification for this Gene Disease Record based on this evidence, please see the section below. Otherwise, click "Cancel" to return to your previous page."
Change to: If you would like to 1) save the new "Calculated Classification" highlighted in blue or 2) change your Last Saved Classification value based on this new "Calculated Classification" value, make sure to edit the highlighted fields and click Save. You may also choose to mark your Classification as Provisional."
Continuing:
"The above Classification Matrix was calculated based on the current evidence and accompanying scores saved in the database when when you clicked the "View Classification Matrix" button to navigate to this page. To save a new Classification based on this current evidence, please fill in the fields below and click "Save". Otherwise, click "Cancel".
@jimmyzhen would it be quick to add word "Calculated" here so it says "Modify Calculated..." (keep linked item same, i.e. don't link "Calculated" in with it: That's what they are really doing each time, I believe.
@jimmyzhen This is working well -- a couple of suggested changes that I hope are quick:
@wrightmw that likely needs work.
Create a table view for all Case Level evidence (Individual evidence primarily since other types of evidence can't be scored) associated with the provisional classification.
Table header title "Genetic Evidence: Case Level (variants, segregation)
Table columns include:
Sortable columns include:
Could they also include: