solgenomics / sgn

The code behind the Sol Genomics Network, Cassavabase and other Breedbase websites
https://solgenomics.net
MIT License
66 stars 35 forks source link

Male parents don't show in new pedigree viewer if they are a population. #1744

Closed bellerbrock closed 6 years ago

bellerbrock commented 6 years ago

Expected Behavior

NCSU is uploading lots of pedigree records with populations as the male parent. These uploads work, but the male parent populations are not showing up the pedigree viewer.

It's important to track the possible males because they will use marker data to calculate each male's genetic distance from the progeny to identify the true male parent. That in turn will lead to better understanding of breeding values, combining ability, and improve parent selection for future crosses.

For Bugs:

Environment

sweetpotatobase.org

Steps to Reproduce

An example record is MC14-0050 , with pedigree NC04-0531/13MC . The population 13MC is not showing in the pedigree viewer

bauchetg commented 6 years ago

@bellerbrock this case seem to deal with the same technical issue as in #1615 @dauglyon correct?

dauglyon commented 6 years ago

@bauchetg @bellerbrock Unfortunately yes, BrAPI doesn't seem to have a population concept, and the pedigree call doesn't support a multiple-parent model, although the pedigree tree viewer can support having more than two parents. @BrapiCoordinatorSelby is this something in the works? If not I'll open a new issue to discuss possible solutions.

BrapiCoordinatorSelby commented 6 years ago

@dauglyon I'm working on #150 for V1.2 which introduces the concept of 'parentType'. This means each pedigree object still only has 2 parent DbIds, but you could use the 'parentType' to refer to a 'population' entity instead of directly to a 'germplasm' entity.

The concept of a 'population' is not present in BrAPI, so go ahead and make a issue for that. If others in the BrAPI community find it useful to support populations, I will be happy to add the concept. Otherwise, sweetpotatobase may need its own 'population' API call which gets called when the ped viewer encounters a 'parentType'='POPULATION'

bellerbrock commented 6 years ago

@dauglyon @BrapiCoordinatorSelby Thanks for the clarification. I wouldn't worry about showing the members of the population in the viewer. For our purposes at least it'd be best to replicate the old viewer's behavior, and show just the pop. name with a link to it's stock detail page.

The old viewer didn't distinguish between the stock types of parents at all, but the addition of 'parentType' to the pedigree call might make it possible to identify and visually differentiate populations from accessions in the viewer.

dauglyon commented 6 years ago

@bellerbrock Also, after version 1 of the new pedigree viewer, I recall being asked to remove children of accessions that were populations/crosses and not individual accessions. Would we only want to show populations when they are parents? Showing populations becomes an issue as it introduces more logic ( i.e. Do we show members? If we don't, how do account for accession which are members of populations if they are the root node of the pedigree graph? etc)

titima15 commented 6 years ago

@dauglyon Hi David, should remove only cross names but not populations.

dauglyon commented 6 years ago

@titima15 Ah, when i removed the crosses I just filtered the data down to only accessions. @bellerbrock it shouldn't be too hard to fix this, though I may need to change our BrAPI calls to allow for them to report populations as parents/children.

dauglyon commented 6 years ago

That being said, it will display accessions and populations as siblings. (e.g. if an an accession is a member of a population, the node for that accession will be shown as a sibling of the population node.

titima15 commented 6 years ago

@dauglyon I also forgot about population as a parent as well ^ ^

bellerbrock commented 6 years ago

@dauglyon @titima15 Ah, I hadn't thought about the possibility of popuations as children. For the purpose of polycrosses they are only needed as parents; I can't think of any use cases where they could be children.

I'm not sure I understand the populations as sibilings issue, but it doesn't sound like a dealbreaker. In fact if it means the possibility of retrieving the members of a population from within the pedigree viewer that could be a feature not a bug.