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

Revisit naming of the 'id' attribute of the 'Coding' class (and change back to 'code') #111

Open
mbrush opened this issue Oct 5, 2022 · 3 comments

Comments

@mbrush
Copy link
Contributor

mbrush commented Oct 5, 2022

The name "id" suggests that the value is a logical id for the Coding object - which IMO it is not.

A Coding is a general purpose complex data type. A Coding object is just a vehicle to carry a code for some concept, and provide metadata about what it means and where it came from. But it is not the same thing as concept/entity represented by the code it holds. For example, a Coding object holding the CURIE MONDO:0007088 is not an instance of Alzheimer's Disease - it is an instance of a Coding that carries a code for the concept of Alzheimer's Disease.

Note that Coding does inherit an id field from Entity, and this is what would hold the logical id for the Coding object (if we wanted to give it one).

Based on this, I don’t think it is correct to name the field in Coding that holds its concept code “id” - at least not as we have envisioned Codings in VA.

@mbrush
Copy link
Contributor Author

mbrush commented Oct 5, 2022

I went back and reviewed the careful notes I took about our earlier decision and think I identified where the logic that led to us choosing the name “id” breaks down.

On the July 27 VA call, everyone initially agreed to use the name "code" - until we looked at how we are recording ids in ValueEntities like Genes, Diseases.

We talked about how you can’t use a Coding within a ValueEntity (because Codings are extensible, and therefore not fit for value object representations). Which means that Codings aren’t directly relevant to the ValueEntity use case.

However, folks also thought that the slot used to ‘identify’ a ExtensibleEntity like a Coding should be the same as the slot used to identify a ValueEntity like a Gene . . . and this should be id, not code

This is fine and good . . . but what I think we missed is that the point of the 'code' field in a Coding is not to identify the Coding object (i.e. it is not the logical id of that object). In fact, we don’t even bother giving the Coding object a logical id b/c it is a Utility Class and doesn’t need one.

DomainEntities don’t use Codings anyway, so what we name that field in a Coding object should have no bearing on ValueObject representation. And I think the types of entities we use Codings to describe are disjoint from the types of things we represent as ValueEntities. So I don’t see any issues with using “code” as we initially decided.

But we should look at where Codings are used in the annotation-source schema, and confirm that this is the case.

@mbrush
Copy link
Contributor Author

mbrush commented Oct 5, 2022

re:

DomainEntities don’t use Codings anyway

I think this may not always be the case.

Do we envision Propositions and other ValueEntities using Codings? If so, they cannot be extensible, right. If not, how represent things that would naturally use Codings?

  • e.g. If we want to include a slot in a Disease value object to describe its mode of inheritance, this would likely be represented using a Coding that takes a HPO 'inheritance pattern' term.
  • Also, ValueObjects that are not DomainEntities (e.g. Propositions) may also have attributes where we would want to use a Coding (e.g. a Molecular Consequence Proposition would probably include an SO term which would be represented as a Coding).

Maybe the thing to do here is just not allow Codings to be extensible. I can't imagine why this would be necessary anyway, and then we could use them in ValueEntity representations.

@larrybabb
Copy link
Contributor

Great points above. Thank you for the in-depth analysis as usual.

I think this shines a light on the tug-of-war we are having with the use of Coding in our model. When we first considered it, at least I, was thinking of the vast use of Coding and CodeableConcept classes in FHIR. As I read through the above, I'm thinking this is one of the reasons they have separate classes like this. Coding does seem like a logical "non-extensible" class that could be used in Value Objects, but CodeableConcept is much more flexible and useful for situations that require it, particularly when a code is not available and a string or label is all an implementer has to fit in that slot.

I'm in favor of making Coding non-extensible, but we need to understand the impact on situations where an implementer does not always have a code for a Coding attribute (in non-value object entities).

@ahwagner ahwagner transferred this issue from ga4gh/gks-metaschema Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants