Feature/framing #1
Conversation
| implementation("io.micronaut.serde:micronaut-serde-jackson") | ||
| implementation("io.micronaut.neo4j:micronaut-neo4j-bolt") | ||
| implementation("org.monarchinitiative.phenol:phenol-core:2.0.3"){ | ||
| exclude group: 'org.slf4j', module: 'slf4j-api' |
There was a problem hiding this comment.
@iimpulse just curious here, why do you exclude slf4j-api?
There was a problem hiding this comment.
I forget lol, I think this came down to some version collision
There was a problem hiding this comment.
In Maven, excluding would not be necessary, because the dependency that is higher in the dependency tree has a precedence. So, if you specify which version you want to use, e.g. in your top-level pom.xml, Maven will use that (more info here if you're interested, see the dependency mediation paragraph)
I think this principle sounds too useful to be Maven-specific. I would expect a similar behavior in Gradle, but 🤷♂️ ...
oan-etl/src/main/java/org/jacksonlaboratory/ontology/GraphLoader.java
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| protected void assayToPhenotype(Session session, File loinc){ |
There was a problem hiding this comment.
Why is this method (and the methods below) protected? Is this due to some framework which I am not aware of? Otherwise, why not to make them private?
Moreover, the methods do not use the class' state, hence they can be further "upgraded" to static.
EDIT: It looks like it's protected to enable testing. Note that you can get the same behavior with package-private visibility, although using the methods from unit tests should not be done in the long run.
There was a problem hiding this comment.
Do you mean testing the load method over the individual pieces?
There was a problem hiding this comment.
Yeah. I usually change the private/package-private bits and then I am forced to change the tests too..
| tx.close(); | ||
| } | ||
|
|
||
| protected void phenotypes(Session session, List<TermId> termIds, Ontology ontology){ |
There was a problem hiding this comment.
This method seems to be persisting HPO term IDs and the corresponding labels. The IDs originate from the HPOA but they are only persisted if there is a corresponding entry in the Ontology.
Why not iterate directly over the ontology terms and store the IDs and labels? That would achieve the same effect, if not better (here the IDs that are in the ontology and not in HPOA will be persisted).
In other words why not just store the Ontology nodes/terms?
| Transaction tx = session.beginTransaction(); | ||
| logger.info("Loading Disease to Phenotype Relationships..."); | ||
| diseases.diseaseData().stream().flatMap(d -> d.annotationLines().stream()).forEach(line -> { | ||
| String onset = line.onset().map(HpoOnset::id).map(TermId::toString).orElse(""); |
There was a problem hiding this comment.
Are the "" (empty string) equivalent to a missing value in NEO4J? I am not familiar with the semantics of the database, but "" and null can convey a different meaning..
The same applies for sex on L119 below.
oan-etl/src/main/java/org/jacksonlaboratory/ontology/HpoGraphLoader.java
Outdated
Show resolved
Hide resolved
| for (TermId termId: termIds){ | ||
| Optional<String> label = ontology.getTermLabel(termId); | ||
| label.ifPresent(s -> tx.run("CREATE (p:Phenotype {id: $id, name: $name})", | ||
| parameters("id", termId.toString(), "name", s))); |
There was a problem hiding this comment.
In TermId, the contract of toString is the same as contract of Display trait in Rust - show the data.
It just happens to be that it returns a good CURIE, but that's subject to change anytime.
Use getValue() if CURIE is what you want. I think that's what you're doing here, so really pls replace toString with getValue everywhere.
oan-etl/src/main/java/org/jacksonlaboratory/ontology/HpoGraphLoader.java
Outdated
Show resolved
Hide resolved
oan-rest/src/main/java/org/jacksonlaboratory/model/SimpleOntologyTerm.java
Outdated
Show resolved
Hide resolved
Two module gradle project in java 17.
oan-etl will load are default ontology data like we do currently in hpo-web to a neo4j database.
oan-rest is a micronaut restful api to query this graph.