search

populationgenomics / metamist

Sample level metadata system
MIT License
1 stars 1 forks source link

Rendering Pedigrees #360

Open danielreti opened 1 year ago

danielreti commented 1 year ago

What is the best way to draw pedigrees?

Write our own pedigree hierarchy and render functions from scratch

Pros Fine control over all aspects Can easily reuse component in future apps Currently works on most common cases, can slowly build out more complicated handling as it arises

Cons All our own maintenance Could be difficult to validate?

Use PedigreeJS

Pros Existing package, maintained relatively regularly-ish Seems to do a goodish job based on some testing

Cons No npm package Uses jQuery No control over style/layout + can't get rid of edit options Some cases we have (eg duos) do not work and the author has no intention of implementing

Fork PedigreeJS / Copy logic with license and repurpose as needed

Pros Can use the PedigreeJS code to create the hierarchy structure and render it in our own style Can easily reuse component in future apps

Cons Will need to migrate from jQuery to ReactJS (not too hard) + validate Need to update when PedigreeJS updates if it's a big fix

Use another package

Not sure which package

Don't show pedigrees

Last resort ;)

lgruen commented 1 year ago

It's also worth noting that seqr uses PedigreeJS already, so having the same pedigree drawings in both systems would be nice.

How does seqr deal with the integration issues that you've run into with PedigreeJS?

danielreti commented 1 year ago

So seqr lists a specific PedigreeJS commit as a dependency and are using the package that way (which is totally fine). I just think things like not being able to have single parents (https://github.com/CCGE-BOADICEA/pedigreejs/issues/120), not being able to modify any of the styling and not passing down click events are reasons enough to not want to use it.

I don't see using our own viewer as being a lot of work or a lot of ongoing maintenance, the function I've written now works quite well I daresay on the requested test sets and we can slowly just layer in more functionality as we need to generate more complex pedigrees.

danielreti commented 1 year ago

Realised you may not have seen my response @lgruen (can't quite tell who GitHub notifies)

lgruen commented 1 year ago

Those are great points, but why not contribute to PedigreeJS to get this implemented instead of coming up with yet another implementation?

danielreti commented 1 year ago

Some of the features appear to be fundamentally embedded in the tree architecture, (ie a node must have 0 or 2 parents), so those would to be solved some other way. The author generally does not seem too receptive to feature requests. I frankly don't think it's worth my time to learn how his implementation works and sending PRs to it.