Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create class hierarchy diagram #561

Open
ahwagner opened this issue Oct 16, 2024 · 12 comments
Open

Create class hierarchy diagram #561

ahwagner opened this issue Oct 16, 2024 · 12 comments
Assignees
Labels
Product Review Features requested as part of product review
Milestone

Comments

@ahwagner
Copy link
Member

From #533:

Please add a figure showing the class hierarchy to illustrate inheritance. This would be especially helpful for an abstract class like Location, which is listed as a datatype for several attributes but is not included in the documentation so it is difficult to know which child classes are valid for a given attribute.

@ahwagner ahwagner added the 2.0 proposed changes and feature requests for v2 label Oct 16, 2024
@larrybabb larrybabb self-assigned this Oct 16, 2024
@ahwagner ahwagner added this to the 2.0 milestone Oct 22, 2024
@ahwagner ahwagner added the Product Review Features requested as part of product review label Oct 22, 2024
@ahwagner ahwagner added this to VRS Oct 22, 2024
@ahwagner ahwagner removed the 2.0 proposed changes and feature requests for v2 label Oct 25, 2024
@ahwagner ahwagner moved this from TODO to In Progress in VRS Oct 25, 2024
@larrybabb
Copy link
Contributor

Here's the class diagram.
GKS Product Organization - VRS 2 0 Class Diagram

@larrybabb
Copy link
Contributor

larrybabb commented Oct 25, 2024

@ahwagner and others.... I'd love any feedback in terms of making the diagram easier to understand. I do think we may need different versions or representations of the class diagram, including more detailed versions focused on individual classes. But for now we are trying to get the basic view down to a single page to give users a sense of the scope and breadth and organization of VRS.

@ahwagner
Copy link
Member Author

This looks good to me. FWIW, I think this is exactly the level of detail needed for this diagram, which covers a lot of ground.

@Mrinal-Thomas-Epic
Copy link
Contributor

This is a great one page overview! I think the only thing that might be helpful to add is a "uses" type relationship (e.g., Allele uses SequenceLocation and SequenceExpression)

@rrfreimuth
Copy link
Contributor

Thanks, @larrybabb ! I think this is great. The "inheritance view" is important and aids understanding.

A single diagram cannot show everything. I agree that another diagram that illustrates more of a structural domain perspective (detail within the classes and relevant relations/compositions) would also be helpful at some point.

@ahwagner
Copy link
Member Author

@larrybabb one update to request: Ga4ghIdentifiableObject is in VRS, not gks.core. We should either move it up to core (I would prefer this), or revise this figure to match the spec. Otherwise I think this looks good.

@ahwagner ahwagner assigned larrybabb and unassigned ahwagner Oct 30, 2024
@larrybabb
Copy link
Contributor

Image

@larrybabb larrybabb assigned ahwagner and unassigned larrybabb Oct 30, 2024
@korikuzma
Copy link
Contributor

@larrybabb how do you decide on when to include ABC? For example, Variation is included, but MolecularVariation, SystemicVariation, and Location are not.

@korikuzma
Copy link
Contributor

@larrybabb LiteralSequenceExpression vrs is black, but should be blue

@korikuzma
Copy link
Contributor

@larrybabb CisPhaseBlock --> CisPhasedBlock

@larrybabb
Copy link
Contributor

@larrybabb how do you decide on when to include ABC? For example, Variation is included, but MolecularVariation, SystemicVariation, and Location are not.

@korikuzma Alex and I decided that only Classes that are strictly organizational do not go in the class hierarchy diagram. Since MolecularVariation and SystemicVariation are more about grouping types of Variation we did not include them in the more technical inheritance class diagram. Does that make sense? (please discuss with Alex as well)

@korikuzma
Copy link
Contributor

@larrybabb how do you decide on when to include ABC? For example, Variation is included, but MolecularVariation, SystemicVariation, and Location are not.

@korikuzma Alex and I decided that only Classes that are strictly organizational do not go in the class hierarchy diagram. Since MolecularVariation and SystemicVariation are more about grouping types of Variation we did not include them in the more technical inheritance class diagram. Does that make sense? (please discuss with Alex as well)

@larrybabb that makes sense. But what about Location, it seems similar to Variation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product Review Features requested as part of product review
Projects
Status: Done
Development

No branches or pull requests

5 participants