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

Subontology #31

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Subontology #31

wants to merge 2 commits into from

Conversation

pnrobinson
Copy link
Collaborator

I have added code that makes the TermMap and the ReleationMap of subontologies created with the function subOntology in ImmuntableOntology containly only there terms and relations of the suibontology (previously, they contained the terms of the original ontology). I have added a line to the metadata that says that the ontology was subsetted.

Copy link
Contributor

@holtgrewe holtgrewe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, overall I don't understand what the problem is. You are changing the behaviour from the Javadoc here:

import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no star imports.

expectedSize=1;
assertEquals(expectedSize,relationMap.size());
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove superflous empty lines.

return new ImmutableOntology<T, R>(metaInfo, subGraph, subOntologyRoot,
Sets.intersection(nonObsoleteTermIds, childTermIds),
Sets.intersection(obsoleteTermIds, childTermIds), termMap, relationMap);
Set<TermId> intersectingTerms = Sets.intersection(nonObsoleteTermIds,childTermIds);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace is inconsistent. Your IDE should be able to help you with auto-formatting.

@pnrobinson
Copy link
Collaborator Author

This Javadoc behavior should be changed

The constructed sub ontology will use the same maps from {@link TermId} to {@code T} and same
   * edge relation maps as the original ontology. However, the functions
   * {@link Ontology#getAllTermIds()}, {@link Ontology#getNonObsoleteTermIds()},
   * {@link Ontology#getObsoleteTermIds()}, and {@link Ontology#getTerms()} will only contain the
   * ids of the sub ontology. The term ID and relation maps might contain more elements in the case
   * of creating sub ontologies and might refer to elements that are not present in the term id sets
   * or the graph.

When client code creates a subontology, e.g., all phenotype terms, then there are many situations where one needs to iterate over all of the terms and it is useful to use the TermMap for this purpose. It is not useful to have an ontology and a term map that are not consistent with one another. I think we should change this behavior

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

Successfully merging this pull request may close these issues.

2 participants