Skip to content
This repository has been archived by the owner on Sep 25, 2018. It is now read-only.

Make an AbstractEntity base class for all Entities #58

Open
julesjacobsen opened this issue May 17, 2016 · 3 comments
Open

Make an AbstractEntity base class for all Entities #58

julesjacobsen opened this issue May 17, 2016 · 3 comments
Assignees

Comments

@julesjacobsen
Copy link
Contributor

(child_of: #56)

AbstractEntity should implement Entity all entity classes should extend AbstractEntity

Entity is_a ClassInstance where ClassInstance is defined as:

An abstract class for anything that can be described as a boolean combination of ontology classes

Which rather covers everything and anything. Usage of ClassInstance is not completely uniform - GenomicEntity and Publication aren't marked as ClassInstance but everything else in condition, entity, environment and evidence packages extend from ClassInstance so most likely these are omissions.

So:

  1. Should GenomicEntity be a ClassInstance (I think yes, and then given that I'll create an AbstractEntity as per this ticket.)
  2. Should Publication be a ClassInstance (maybe? Probably yes.)
  3. Should all classes extending ClassInstance have their own @JsonldType (Probably. Again, this is sporadic in the code, but then we have issues of mismatch i.e. org.phenopackets.evidence.Publication could be associated with IAO:0000311 but is probably better less constrained as a IAO:0000310 however if not then it can be better described in ClassInstance.types)

@cmungall, @selewis, @balhoff - opinions?

@balhoff
Copy link
Member

balhoff commented Jun 21, 2016

It seems like something should be a class instance only if you want to be able to provide a list of types and negated types. You probably wouldn't need that for Publication, right?

Currently I don't think we are doing anything with the @JsonldType annotations. We don't output @type properties for anything.

@cmungall
Copy link
Member

negated types for publications is overkill, but it's still a class instance.

Yes, we're not using @JsonldType.

@julesjacobsen
Copy link
Contributor Author

julesjacobsen commented Jul 27, 2016

Might want to have a OntologyClass.notOf(id, label) builder for general negated types. Can then ask the OntologyClass if it's negated intead of adding it to a negated list.

Also consider using Decorator where Entity is decorated instead of all this inheritance on an AbstractEntity.

@julesjacobsen julesjacobsen self-assigned this Jul 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants