ga4gh / gks-common

Common classes and schemas used by all GKS specifications (ie. VR, VA, etc..)
Apache License 2.0
2 stars 0 forks source link

Add a 'type' field to Entity? #5

Closed mbrush closed 2 months ago

mbrush commented 4 months ago

Seems like the `type' property should be applicable to any Entity subtype?

At present, it is added separately to most (but not all) subtypes of this class. Was this by design? If so, what is rationale? If not, can we move this property to live in the Entity class, rather than define separately to specific subclasses?

Related: In the core-im-source yaml file, I noted that some classes have a type property with a hard coded value (e.g. "Document", "DataItem"), and others do not have a hard-coded type (e.g. "DataSet", "Statement") – by design, or should we fix/make consistent. I will make a PR to add missing hard-coded types.

larrybabb commented 3 months ago

@ahwagner I think it is reasonable and pragmatic to move type to the top level Entity as a required field and remove any subclasses that should not have type (i.e. Mapping).

NOTE: currently MappableEntity does not have type but I believe it should.

larrybabb commented 2 months ago

PR #36