clingen-data-model / clingen-interpretation

Allele (variant) interpretation model and API for ClinGen
3 stars 1 forks source link

Combine Inherited attributes w/ direct attributes in generated Markup files #230

Closed larrybabb closed 6 years ago

larrybabb commented 6 years ago

@bpow It would be helpful to combine all attributes, inherited or direct into a single "Attributes" table on the individually generated entity pages for our documentation.

The Attribute.precedence column should be used to define the order of the attributes in the table (which may already be done).

If you can add a column at the end of the attributes table to indicate whether or not the attribute was inherited it would be a bonus. Not absolutely required however.

bpow commented 6 years ago

Hmmm... This is not so much a combine as it is "return inherited attributes to the documentation, but put them in the same table as the direct attributes". I hadn't noticed that you had removed the inherited attributes in 4169a339429 (I guess that was part of the "misc cleanup" referenced in the commit message).

I do think that it is really important to have the inherited attributes as part of the documentation. My thinking in having direct and inherited attributes in separate tables is along what is done in, for instance, javadoc-- the "direct" attributes are more central to the definition of the specific object. I can put them in one table, though, and see what it looks like.

bpow commented 6 years ago

See what you think about this in e4e5215 ... I added a "defined in" column since I think that helps when trying to understand the model.

The order is a bit weird-- I put the items defined in the current type at the top (since they are the most important in the context of a given page).

Also-- we can discuss whether to include label/description (since we've talked about Entity as a pseudo-type that we may be hiding). The downside of not having them here would be a person, first approaching the model, thinking "where did those come from?"

bpow commented 6 years ago

@larrybabb : I'd like to know more about your rationale for having direct and inherited attributes in one table. Thinking back on / looking at javadoc as an example, they further de-emphasize the inherited attributes by just listing them as comma-separated in text, referring to the type in which they are defined for more formal definition. We could do something like this, or even have the "inherited attributes" in a separate tab, hidden at first behind the tab for the direct attributes.

larrybabb commented 6 years ago

Let's leave it as a separate thing. But we will have to have a page for "Entity" to point to IMO.

Going back and forth between trying to show the "model" (in UML classic sense) and JSON is challenging IMO. FHIR does a pretty excellent job of accomplishing what it is we are trying to accomplish with the documentation.

I thought at one point I saw that FHIR's Resources were including the inherited attributes from DomainResource and Resource itself but they are not. Here's what they are doing (here's the Patient resource link to view live)

image

As you can see in the highlighted area they do mention the direct inherited attributes from the DomainResource at the top level description of the "type" of the resource itself.

Our docs don't really do this, but maybe we should. It also would help to have an Entity page to bounce to to explain where these things are coming from.

Let's do whatever is easiest for now (putting back "inherited attributes" table) and then create an issue to rectify once-and-for-all later.

larrybabb commented 6 years ago

done