monarch-initiative / phenogrid

The phenogrid widget
13 stars 14 forks source link

apiEntityMap needs to be updated for genotype #210

Closed yuanzhou closed 8 years ago

yuanzhou commented 8 years ago

I'm working on genotype expansion for phenogrid. Once all the matches returned from the http://beta.monarchinitiative.org/compare API, I'll need to add them into the target data list, and each genotype will also need a type attribute to be specified.

Right now we are using the apiEntityMap's apifragment property as the desired type. Some genotypes have the same 'MGI' prefix, what should I do to add the genotype type? Otherewise I can't distinguish genotype from gene. @harryhoch

And some genotypes have the MONARCH prefix instead of MGI.

apiEntityMap: [ 
                {prefix: "HP", apifragment: "disease"},
                {prefix: "OMIM", apifragment: "disease"}, 
                {prefix: "ZFIN", apifragment: "gene"},          
                {prefix: "MGI", apifragment: "gene"}
            ]
harryhoch commented 8 years ago

@yuanzhou, Good questions. I'm not sure that I know the answers. @kshefchek ?

kshefchek commented 8 years ago

Typically it's not possible to determine ID type based on prefix (HP excepted in that list). I could add type to any json object, where is this needed?

yuanzhou commented 8 years ago

This is used in the dataLoader: https://github.com/monarch-initiative/phenogrid/blob/master/js/dataloader.js#L167

I think I could get around this by not using this apiEntityMap but manually add genotype as the type of each genotype. Aside from this, the apiEntityMap is only used by the dataLoader to determine the type of other targets. And this type info is also used in the tooltip of grid cell.

Maybe I should get rid of this apiEntityMap from phenogrid, but this assumes that mouse and fish are only use gene, human uses disease. Any suggestions?

kshefchek commented 8 years ago

Is the issue that owlsim does not return type information?

yuanzhou commented 8 years ago

There are id and label in every return now. It will be great if we add type there too, I'm not sure if every id/label really has a unique type. E.g., MGI can be used for either gene or genotype. If we add the type info along with the ID, at least it resolves this issue.

kshefchek commented 8 years ago

Let me see if this is something I can address on the Monarch API level.

yuanzhou commented 8 years ago

Thanks @kshefchek please keep me posted on this. If it can not be addressed on the monarch api level, I'll try to find a workaround and get rid the use of this.

yuanzhou commented 8 years ago

@harryhoch, not sure how long I'll wait until @kshefchek has this issue addressed in the API level. Can I assume the type of targets under human is always 'disease', and type of targets under fish/mouse is always 'gene' or 'genotype'? Because right now the genotypes are being added after all the species targets are loaded. If my assumption is true, I can simply specify the type based on the species name instead of the prefix. And I can specify the type of all the added genotypes as 'genotype' later, since only fish/mouse has this feature for now.

harryhoch commented 8 years ago

@yuanzhou, I think this is a fine approach for now.

nlwashington commented 8 years ago

FYI, i hesitate to add any assumptions to logic within phenogrid. it should not know what a "disease" is.

phenogrid should be generic enough to render anything from human with the HP prefix. some things that are annotated with HP are diseases (as is what you are getting from owlsim at the moment), but in the future it could be anything: genes, genotypes, variants, people. please, please don't hardcode anything!

nlwashington commented 8 years ago

furthermore, i do think that you ought to be given the types in some way; whether that is from some kind of monarch-api call, or from owlsim directly, you need the info. at the moment, owlsim doesn't store anything about types and it'll be some time until that's refactored/added; but since you are getting the genotypes from monarch app, the schema could include a "type" : genotype for example, together with the id and label. if you have a preference for how this looks, please specify.

harryhoch commented 8 years ago

@yuanzhou, @nlwashington makes an excellent point here. Please sketch out what you would need to make this code sufficiently generic and flexible. Let's not hardcode anything that we can parameterize...

kshefchek commented 8 years ago

I think this will be fixed with my PR or am I misunderstanding? https://github.com/monarch-initiative/monarch-app/pull/1031

yuanzhou commented 8 years ago

@kshefchek I was aware of your PR fix. Will make the changes once your PR get merged. Thanks!

yuanzhou commented 8 years ago

@kshefchek after your PR https://github.com/monarch-initiative/monarch-app/pull/1031 got merged, I could see the type in the compare API returned JOSN. But the type is still null from the initial simsearch POST.

http://beta.monarchinitiative.org/simsearch/phenotype

with the following POST data:

input_items=HP:0000726+HP:0000746+HP:0001300+HP:0002367+HP:0000012+HP:0000716+HP:0000739+HP:0001332+HP:0001347+HP:0002063+HP:0002067+HP:0002172+HP:0002322+HP:0007159+HP:0009466+HP:0001972+HP:0003502+HP:0005190+HP:0001763+HP:0003318+HP:0001371+HP:0001380+HP:0001394+HP:0001156+HP:0001159+HP:0001004+HP:0010886+HP:0006530+HP:0004792+HP:0002816+HP:0000954+HP:0000974+HP:0000765+HP:0000602+HP:0002240+HP:0030084+HP:0000049+HP:0000028+HP:0000175+HP:0000204+HP:0000202+HP:0000316+HP:0000343+HP:0000349+HP:0002023+HP:0000327+HP:0002055+HP:0000394+HP:0000463+HP:0000431+HP:0000494+HP:0000484+HP:0000486+HP:0000508+HP:0009748+HP:0007018+HP:0001773+HP:0001769+HP:0003311+HP:0001544+HP:0001508+HP:0003196+HP:0001249+HP:0001187+HP:0001169+HP:0012774+HP:0002650&target_species=7955
kshefchek commented 8 years ago

Made a small tweak and cleared the cache, it should be working when beta updates.

yuanzhou commented 8 years ago

@kshefchek Tested again and it worked! Thanks a lot!