diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 083f78e39..c6604da40 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -21,9 +21,9 @@ jobs: steps: - uses: actions/checkout@v2 - name: Set up JDK 17 - uses: actions/setup-java@v2 + uses: actions/setup-java@v5.2.0 with: java-version: '17' distribution: 'adopt' - name: Build with Maven - run: mvn --batch-mode --update-snapshots verify + run: ./mvnw --batch-mode --update-snapshots verify diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index e02b50593..11174129d 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -14,15 +14,15 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v6.0.2 with: fetch-depth: 0 # otherwise, you will fail to push refs to dest repo ref: main - name: Set up Python 3. - uses: actions/setup-python@v4 + uses: actions/setup-python@v6.2.0 with: - python-version: 3.9 + python-version: 3.12 - name: Install Python dependencies run: | @@ -30,7 +30,7 @@ jobs: pip install sphinx-rtd-theme - name: Set up JDK 17 - uses: actions/setup-java@v3 + uses: actions/setup-java@v5.2.0 with: java-version: '17' distribution: 'adopt' @@ -100,7 +100,7 @@ jobs: - name: Deploy documentation. if: ${{ github.event_name == 'push' }} - uses: JamesIves/github-pages-deploy-action@v4.4.1 + uses: JamesIves/github-pages-deploy-action@v4.8.0 with: branch: gh-pages force: true diff --git a/.gitignore b/.gitignore index acef8ce9b..287c85bc8 100644 --- a/.gitignore +++ b/.gitignore @@ -75,3 +75,5 @@ families/* phenopackets/* cohorts/* +# Python stuff +venv/ diff --git a/.mvn/wrapper/maven-wrapper.properties b/.mvn/wrapper/maven-wrapper.properties index 0061e751e..72853742a 100644 --- a/.mvn/wrapper/maven-wrapper.properties +++ b/.mvn/wrapper/maven-wrapper.properties @@ -1 +1 @@ -distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.6.0/apache-maven-3.6.0-bin.zip \ No newline at end of file +distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.6.3/apache-maven-3.6.3-bin.zip \ No newline at end of file diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2015ee0c3..c33478258 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -9,7 +9,7 @@ Changelog * Improve user guide, tutorial, and documentation. * Introduce *latest* and *stable* documentation branches * Add showcase of apps that use phenopacket-tools to add more code examples -* Run `MetaDataValidator` during base validation +* Run `MetaDataValidator` and `HpoUniqueValidator` during base validation * Finalize JSON and YAML format sniffing * Fix example phenopackets diff --git a/README.md b/README.md index 54bf0ae84..8162ea203 100644 --- a/README.md +++ b/README.md @@ -33,15 +33,15 @@ a *Library user guide*, and the *Javadoc API documentation*. The documentation is published in two documentation branches: - **stable**: corresponds to the latest published release, and generally also to the last commit of the `main` Git branch - - [Tutorial](http://phenopackets.org/phenopacket-tools/stable/tutorial.html) - - [CLI user guide](http://phenopackets.org/phenopacket-tools/stable/cli.html) - - [Library user guide](http://phenopackets.org/phenopacket-tools/stable) - - [Javadoc API documentation](http://phenopackets.org/phenopacket-tools/stable/apidocs) + - [Tutorial](https://phenopackets.github.io/phenopacket-tools/stable/tutorial.html) + - [CLI user guide](https://phenopackets.github.io/phenopacket-tools/stable/cli.html) + - [Library user guide](https://phenopackets.github.io/phenopacket-tools/stable) + - [Javadoc API documentation](https://phenopackets.github.io/phenopacket-tools/stable/apidocs) - **latest**: corresponds to the bleeding edge code that is on the `develop` Git branch - - [Tutorial](http://phenopackets.org/phenopacket-tools/latest/tutorial.html) - - [CLI user guide](http://phenopackets.org/phenopacket-tools/latest/cli.html) - - [Library user guide](http://phenopackets.org/phenopacket-tools/latest) - - [Javadoc API documentation](http://phenopackets.org/phenopacket-tools/latest/apidocs) + - [Tutorial](https://phenopackets.github.io/phenopacket-tools/latest/tutorial.html) + - [CLI user guide](https://phenopackets.github.io/phenopacket-tools/latest/cli.html) + - [Library user guide](https://phenopackets.github.io/phenopacket-tools/latest) + - [Javadoc API documentation](https://phenopackets.github.io/phenopacket-tools/latest/apidocs) ## Issues diff --git a/docs/conf.py b/docs/conf.py index 4997f4605..544fecd98 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -6,22 +6,6 @@ # -- Project information ----------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#project-information -############# -# JR added: -import os -import sys - -sys.path.insert(0, os.path.abspath('../../')) -extensions = [ - 'sphinx.ext.autodoc', - 'sphinx.ext.githubpages', - 'sphinx_rtd_theme', - 'recommonmark' -] - -html_theme = 'sphinx_rtd_theme' -############## - project = 'phenopacket-tools' copyright = '2022, Daniel Danis, Peter Robinson' author = u'Daniel Danis, Peter Robinson' @@ -33,7 +17,7 @@ # The short X.Y version. version = '1.0' # The full version, including alpha/beta/rc tags. -release = '1.0.0-RC3' +release = '1.0.0-RC4-SNAPSHOT' # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration @@ -48,7 +32,6 @@ # -- Options for HTML output ------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output -html_theme = 'alabaster' html_static_path = ['_static'] html_css_files = ['pxftools.css'] diff --git a/docs/converting.rst b/docs/converting.rst index c1f5842be..99d2c4bcd 100644 --- a/docs/converting.rst +++ b/docs/converting.rst @@ -44,5 +44,5 @@ See also ^^^^^^^^ The API documentation of the conversion functionality is located in the -`org.phenopackets.phenopackettools.converter `_ +`org.phenopackets.phenopackettools.converter `_ module. diff --git a/docs/creating.rst b/docs/creating.rst index 918acb6d9..b8bb97558 100644 --- a/docs/creating.rst +++ b/docs/creating.rst @@ -12,7 +12,7 @@ of the phenopacket framework will want to use a recommended set of ontology term the equivalent message. This section exemplifies usage of `PhenotypicFeatureBuilder`, one of many builders provided by the -`org.phenopackets.phenopackettools.builder `_ +`org.phenopackets.phenopackettools.builder `_ module. @@ -89,12 +89,12 @@ See also ======== See the API documentation of the -`org.phenopackets.phenopackettools.builder `_ +`org.phenopackets.phenopackettools.builder `_ module for a comprehensive list of ontology constants, convenience methods, and builders provided by the *phenopacket-tools* library. Several detailed examples are available in the ``phenopackets-tools-cli`` module in the -`org.phenopackets.phenopackettools.cli.examples `_ +`org.phenopackets.phenopackettools.cli.examples `_ package. .. _Protocol Buffer (protobuf): https://developers.google.com/protocol-buffers diff --git a/docs/validation.rst b/docs/validation.rst index 404ba3f96..1f8089dc0 100644 --- a/docs/validation.rst +++ b/docs/validation.rst @@ -185,7 +185,7 @@ and the discovered issues (if any): The API documentation of the core validation API can be found in the -`org.phenopackets.phenopackettools.validator.core `_ +`org.phenopackets.phenopackettools.validator.core `_ module. @@ -327,7 +327,7 @@ because an annotation with a specific HPO term (e.g., a patient with perimembran In this section, we describe validators that use HPO file to perform several checks that can be useful in many contexts. The API documentation and the corresponding Java classes can be found in -`org.phenopackets.phenopackettools.validator.core.phenotype `_ +`org.phenopackets.phenopackettools.validator.core.phenotype `_ package. @@ -353,6 +353,20 @@ library to parse the HPO JSON file. The `OntologyLoader` is part of `phenol-io `_ module, you may need to add an appropriate dependency into your build file. +.. _rstuniquephenotypevalidation: + +Unique phenotypic features +########################## + +The `HpoUniqueValidator` checks if the HPO terms used by a phenopacket are *unique* - present at most once. +If a term is not unique, the validator points this out along with the term's occurrence count. + +In code, we add the corresponding validator into the validation workflow by running: + +.. code-block:: java + + builder.addValidator(HpoPhenotypeValidators.Unique.phenopacketValidator(hpo)); + .. _rstancestryphenotypevalidation: @@ -432,7 +446,7 @@ See also ^^^^^^^^ Check out the -`org.phenopackets.phenopackettools.validator.core `_ +`org.phenopackets.phenopackettools.validator.core `_ and -`org.phenopackets.phenopackettools.validator.jsonschema `_ +`org.phenopackets.phenopackettools.validator.jsonschema `_ modules for more information regarding the public validation API. diff --git a/phenopacket-tools-builder/pom.xml b/phenopacket-tools-builder/pom.xml index e7594d8e4..e7a470d93 100644 --- a/phenopacket-tools-builder/pom.xml +++ b/phenopacket-tools-builder/pom.xml @@ -7,7 +7,7 @@ org.phenopackets.phenopackettools phenopacket-tools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT phenopacket-tools-builder diff --git a/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VariationDescriptorBuilder.java b/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VariationDescriptorBuilder.java index 55bcc5a29..96fb3eb07 100644 --- a/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VariationDescriptorBuilder.java +++ b/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VariationDescriptorBuilder.java @@ -100,6 +100,10 @@ public VariationDescriptorBuilder transcript() { return this; } + public VariationDescriptorBuilder moleculeContext(MoleculeContext context) { + builder.setMoleculeContext(context); + return this; + } public VariationDescriptorBuilder structuralType(OntologyClass structuralType) { builder.setStructuralType(structuralType); diff --git a/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VcfRecordBuilder.java b/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VcfRecordBuilder.java index 472c56b8c..c0b58fb4c 100644 --- a/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VcfRecordBuilder.java +++ b/phenopacket-tools-builder/src/main/java/org/phenopackets/phenopackettools/builder/builders/VcfRecordBuilder.java @@ -2,9 +2,14 @@ import org.ga4gh.vrsatile.v1.VcfRecord; +import java.util.ArrayList; +import java.util.List; + public class VcfRecordBuilder { private final VcfRecord.Builder builder; + private boolean passIsSet = false; + private List filters = null; private VcfRecordBuilder(String assembly, String chromosome, int position, String ref, String alt) { builder = VcfRecord.newBuilder() @@ -48,18 +53,46 @@ public VcfRecordBuilder qual(String qual) { } /** - * If this method is called, "PASS" is added to the FILTER column + * If this method is called, the FILTER column is set to "PASS" and any other previously added filters are cleared. + * Calling {@code pass()} is equivalent to calling filter("PASS"). */ - public VcfRecordBuilder pass() { - builder.setFilter("PASS"); + public synchronized VcfRecordBuilder pass() { + passIsSet = true; + if (filters != null) + filters.clear(); + return this; } /** - * @param filter FILTER field of VCF. calling {@link #pass()} is equivant to calling filter("PASS) + * Add a VCF filter field. + *

+ * The field can be a single value (e.g. q50) or several values joined by ; (e.g. q10;q50). + *

+ * Calling filter("PASS") is equivalent to calling {@link #pass()}. As a side effect, + * all previously added filter values are removed. + * + * @param filter add a FILTER field. + * @see #pass() */ - public VcfRecordBuilder filter(String filter) { - builder.setFilter(filter); + public synchronized VcfRecordBuilder filter(String filter) { + passIsSet = false; + if (filters == null) + filters = new ArrayList<>(); + + if ("PASS".equalsIgnoreCase(filter)) + return pass(); + else { + if (filter.contains(";")) { + for (String field : filter.split(";")) { + String trimmed = field.trim(); + if (!trimmed.isEmpty()) + this.filter(field); + } + } else + filters.add(filter); + } + return this; } @@ -71,7 +104,13 @@ public VcfRecordBuilder info(String info) { return this; } - public VcfRecord build() { + public synchronized VcfRecord build() { + if (passIsSet) + builder.setFilter("PASS"); + + else if (filters != null && !filters.isEmpty()) + builder.setFilter(String.join(";", filters)); + return builder.build(); } } diff --git a/phenopacket-tools-builder/src/test/java/org/phenopackets/phenopackettools/builder/builders/VcfRecordBuilderTest.java b/phenopacket-tools-builder/src/test/java/org/phenopackets/phenopackettools/builder/builders/VcfRecordBuilderTest.java new file mode 100644 index 000000000..b044187a8 --- /dev/null +++ b/phenopacket-tools-builder/src/test/java/org/phenopackets/phenopackettools/builder/builders/VcfRecordBuilderTest.java @@ -0,0 +1,30 @@ +package org.phenopackets.phenopackettools.builder.builders; + +import org.ga4gh.vrsatile.v1.VcfRecord; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import static org.junit.jupiter.api.Assertions.*; +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; + +public class VcfRecordBuilderTest { + + @ParameterizedTest + @CsvSource({ + "PASS, PASS", + "pass, PASS", + "q50, q50", + "q50;q10, q50;q10", + "q50;pass, PASS", + "pass;q50, q50", + }) + public void addFilter(String filter, String expected) { + VcfRecord record = VcfRecordBuilder.builder("GRCh37", "chr1", 123_456, "C", "G") + .filter(filter) + .build(); + + assertThat(record.getFilter(), equalTo(expected)); + } +} \ No newline at end of file diff --git a/phenopacket-tools-cli/pom.xml b/phenopacket-tools-cli/pom.xml index 871fc4116..6a151bb82 100644 --- a/phenopacket-tools-cli/pom.xml +++ b/phenopacket-tools-cli/pom.xml @@ -7,7 +7,7 @@ org.phenopackets.phenopackettools phenopacket-tools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT phenopacket-tools-cli diff --git a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/ValidateCommand.java b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/ValidateCommand.java index 4dd8b3b0f..af9725847 100644 --- a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/ValidateCommand.java +++ b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/ValidateCommand.java @@ -3,8 +3,8 @@ import com.google.protobuf.MessageOrBuilder; import org.monarchinitiative.phenol.base.PhenolRuntimeException; -import org.monarchinitiative.phenol.io.OntologyLoader; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.io.MinimalOntologyLoader; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.phenopackettools.core.PhenopacketElement; import org.phenopackets.phenopackettools.core.PhenopacketSchemaVersion; @@ -149,10 +149,10 @@ private List> configureSema // Right now we only have one semantic validator, but we'll extend this in the future. LOGGER.debug("Configuring semantic validators"); List> validators = new ArrayList<>(); - Ontology hpo = null; + MinimalOntology hpo = null; if (validateSection.hpJson != null) { LOGGER.debug("Reading HPO from {}", validateSection.hpJson.toAbsolutePath()); - hpo = OntologyLoader.loadOntology(validateSection.hpJson.toFile()); + hpo = MinimalOntologyLoader.loadOntology(validateSection.hpJson.toFile()); // The entire logic of this command stands and falls on correct state of `element` and the read message(s). // This method requires an appropriate combination of `T` and `element`, as described in Javadoc. @@ -163,18 +163,24 @@ private List> configureSema //noinspection unchecked validators.add((PhenopacketValidator) HpoPhenotypeValidators.Primary.phenopacketHpoPhenotypeValidator(hpo)); //noinspection unchecked + validators.add((PhenopacketValidator) HpoPhenotypeValidators.Unique.phenopacketValidator(hpo)); + //noinspection unchecked validators.add((PhenopacketValidator) HpoPhenotypeValidators.Ancestry.phenopacketHpoAncestryValidator(hpo)); } case FAMILY -> { //noinspection unchecked validators.add((PhenopacketValidator) HpoPhenotypeValidators.Primary.familyHpoPhenotypeValidator(hpo)); //noinspection unchecked + validators.add((PhenopacketValidator) HpoPhenotypeValidators.Unique.familyValidator(hpo)); + //noinspection unchecked validators.add((PhenopacketValidator) HpoPhenotypeValidators.Ancestry.familyHpoAncestryValidator(hpo)); } case COHORT -> { //noinspection unchecked validators.add((PhenopacketValidator) HpoPhenotypeValidators.Primary.cohortHpoPhenotypeValidator(hpo)); //noinspection unchecked + validators.add((PhenopacketValidator) HpoPhenotypeValidators.Unique.cohortValidator(hpo)); + //noinspection unchecked validators.add((PhenopacketValidator) HpoPhenotypeValidators.Ancestry.cohortHpoAncestryValidator(hpo)); } } @@ -191,7 +197,7 @@ private List> configureSema return validators; } - private static PhenopacketValidator prepareOrganSystemValidator(Ontology hpo, + private static PhenopacketValidator prepareOrganSystemValidator(MinimalOntology hpo, List organSystems, PhenopacketElement element) { // Organ system validation can only be done when HPO is provided. diff --git a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/BaseValidateCommand.java b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/BaseValidateCommand.java index d4ecad43f..11b487959 100644 --- a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/BaseValidateCommand.java +++ b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/BaseValidateCommand.java @@ -1,8 +1,8 @@ package org.phenopackets.phenopackettools.cli.command.validate; import com.google.protobuf.MessageOrBuilder; -import org.monarchinitiative.phenol.io.OntologyLoader; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.io.MinimalOntologyLoader; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.ValidationResult; import org.phenopackets.phenopackettools.validator.core.ValidationResults; @@ -109,7 +109,7 @@ protected List> configureSemanticValidators() { List> validators = new ArrayList<>(); if (hpJson != null) { LOGGER.debug("Reading HPO from '{}}'", hpJson.toAbsolutePath()); - Ontology hpo = OntologyLoader.loadOntology(hpJson.toFile()); + MinimalOntology hpo = MinimalOntologyLoader.loadOntology(hpJson.toFile()); validators.add(createHpoValidator(hpo)); } @@ -117,6 +117,6 @@ protected List> configureSemanticValidators() { return validators; } - protected abstract PhenopacketValidator createHpoValidator(Ontology hpo); + protected abstract PhenopacketValidator createHpoValidator(MinimalOntology hpo); } diff --git a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateCohortCommand.java b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateCohortCommand.java index 6355f625f..5e6f00d4a 100644 --- a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateCohortCommand.java +++ b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateCohortCommand.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.cli.command.validate; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.ValidationWorkflowRunner; import org.phenopackets.phenopackettools.validator.core.phenotype.HpoPhenotypeValidators; @@ -30,7 +30,7 @@ protected ValidationWorkflowRunner prepareValidationWorkflow(Li } @Override - protected PhenopacketValidator createHpoValidator(Ontology hpo) { + protected PhenopacketValidator createHpoValidator(MinimalOntology hpo) { return HpoPhenotypeValidators.cohortHpoPhenotypeValidator(hpo); } diff --git a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateFamilyCommand.java b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateFamilyCommand.java index 25390c06b..05481da15 100644 --- a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateFamilyCommand.java +++ b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidateFamilyCommand.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.cli.command.validate; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.ValidationWorkflowRunner; import org.phenopackets.phenopackettools.validator.core.phenotype.HpoPhenotypeValidators; @@ -30,7 +30,7 @@ protected ValidationWorkflowRunner prepareValidationWorkflow(Li } @Override - protected PhenopacketValidator createHpoValidator(Ontology hpo) { + protected PhenopacketValidator createHpoValidator(MinimalOntology hpo) { return HpoPhenotypeValidators.familyHpoPhenotypeValidator(hpo); } diff --git a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidatePhenopacketCommand.java b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidatePhenopacketCommand.java index a16486d43..ceffd9083 100644 --- a/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidatePhenopacketCommand.java +++ b/phenopacket-tools-cli/src/main/java/org/phenopackets/phenopackettools/cli/command/validate/ValidatePhenopacketCommand.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.cli.command.validate; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.ValidationWorkflowRunner; import org.phenopackets.phenopackettools.validator.core.metadata.MetaDataValidators; @@ -32,7 +32,7 @@ protected ValidationWorkflowRunner prepareValidationWorkfl } @Override - protected PhenopacketValidator createHpoValidator(Ontology hpo) { + protected PhenopacketValidator createHpoValidator(MinimalOntology hpo) { return HpoPhenotypeValidators.phenopacketHpoPhenotypeValidator(hpo); } diff --git a/phenopacket-tools-converter/pom.xml b/phenopacket-tools-converter/pom.xml index 2ec70089e..f5c8b2403 100644 --- a/phenopacket-tools-converter/pom.xml +++ b/phenopacket-tools-converter/pom.xml @@ -7,7 +7,7 @@ org.phenopackets.phenopackettools phenopacket-tools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT phenopacket-tools-converter diff --git a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterImpl.java b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterImpl.java index cf80e5d38..e044d8334 100644 --- a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterImpl.java +++ b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterImpl.java @@ -1,5 +1,6 @@ package org.phenopackets.phenopackettools.converter.converters; +import org.ga4gh.vrsatile.v1.MoleculeContext; import org.ga4gh.vrsatile.v1.VariationDescriptor; import org.phenopackets.phenopackettools.builder.builders.*; import org.phenopackets.phenopackettools.converter.converters.v2.*; @@ -209,7 +210,7 @@ private static Optional toV2Interpretation(org.phenopackets.sche diagnosis.addGenomicInterpretation(genomicInterpretation.build()); } - return Optional.ofNullable(InterpretationBuilder.builder(v1.getId()) + return Optional.of(InterpretationBuilder.builder(v1.getId()) .solved(diagnosis.build())); } @@ -220,8 +221,10 @@ private static Function toVariationDescriptor() { return switch (variant.getAlleleCase()) { case HGVS_ALLELE -> { var hgvsAllele = variant.getHgvsAllele(); + Optional moleculeContext = guessMoleculeContext(hgvsAllele.getHgvs()); yield VariationDescriptorBuilder.builder(hgvsAllele.getId()) .zygosity(v2zygosity) + .moleculeContext(moleculeContext.orElse(MoleculeContext.unspecified_molecule_context)) .hgvs(hgvsAllele.getHgvs()) .build(); } @@ -231,6 +234,7 @@ private static Function toVariationDescriptor() { vcfAllele.getChr(), vcfAllele.getPos(), vcfAllele.getRef(), vcfAllele.getAlt()) + .info(vcfAllele.getInfo()) .build(); yield VariationDescriptorBuilder.builder(vcfAllele.getId()) .vcfRecord(vcfRecord) @@ -247,6 +251,7 @@ private static Function toVariationDescriptor() { spdiAllele.getInsertedSequence()); yield VariationDescriptorBuilder.builder(spdiAllele.getId()) .spdi(spdi) + .genomic() .zygosity(v2zygosity) .build(); } @@ -254,6 +259,7 @@ private static Function toVariationDescriptor() { var iscnAllele = variant.getIscnAllele(); yield VariationDescriptorBuilder.builder(iscnAllele.getId()) .iscn(iscnAllele.getIscn()) + .genomic() .zygosity(v2zygosity) .build(); } @@ -263,4 +269,17 @@ private static Function toVariationDescriptor() { }; } + private static Optional guessMoleculeContext(String hgvs) { + if (hgvs.startsWith("NM_") || hgvs.startsWith("XM_") || hgvs.startsWith("ENST")) { + return Optional.of(MoleculeContext.transcript); + } else if (hgvs.startsWith("NP_") || hgvs.startsWith("XP_") || hgvs.startsWith("ENSP")) { + return Optional.of(MoleculeContext.protein); + } else if (hgvs.startsWith("NC_")) { + return Optional.of(MoleculeContext.genomic); + } else { + LOGGER.debug("Could not determine molecule context {transcript, protein, genomic} from HGVS string {}", hgvs); + return Optional.empty(); + } + } + } diff --git a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/AgeConverter.java b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/AgeConverter.java index 2a4c6a9dc..8e142c630 100644 --- a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/AgeConverter.java +++ b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/AgeConverter.java @@ -29,12 +29,11 @@ static Optional toAgeRange(org.phenopackets.schema.v1.core.AgeRange v1 if (start.isEmpty() && end.isEmpty()) return Optional.empty(); - else - return Optional.of( - AgeRange.newBuilder() - .setStart(start.orElse(Age.getDefaultInstance())) - .setEnd(end.orElse(Age.getDefaultInstance())) - .build() - ); + else { + AgeRange.Builder builder = AgeRange.newBuilder(); + start.ifPresent(builder::setStart); + end.ifPresent(builder::setEnd); + return Optional.of(builder.build()); + } } } diff --git a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/DiseaseConverter.java b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/DiseaseConverter.java index 4aead25d2..095f7251e 100644 --- a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/DiseaseConverter.java +++ b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/DiseaseConverter.java @@ -19,27 +19,29 @@ public static List toDiseases(List toDisease(org.phenopackets.schema.v1.core.Disease v1Disease) { + private static Optional toDisease(org.phenopackets.schema.v1.core.Disease v1Disease) { if (v1Disease.equals(org.phenopackets.schema.v1.core.Disease.getDefaultInstance())) return Optional.empty(); Optional term = OntologyClassConverter.toOntologyClass(v1Disease.getTerm()); + Optional onset = toDiseaseOnset(v1Disease); List stages = OntologyClassConverter.toOntologyClassList(v1Disease.getDiseaseStageList()); List tnm = OntologyClassConverter.toOntologyClassList(v1Disease.getTnmFindingList()); - Optional onset = toDiseaseOnset(v1Disease); - if (term.isEmpty() && stages.isEmpty() && tnm.isEmpty() && onset.isEmpty()) + if (term.isEmpty() && onset.isEmpty() && stages.isEmpty() && tnm.isEmpty()) return Optional.empty(); - return Optional.of(Disease.newBuilder() - .setTerm(term.orElse(OntologyClass.getDefaultInstance())) - .addAllDiseaseStage(stages) - .addAllClinicalTnmFinding(tnm) - .setOnset(onset.orElse(TimeElement.getDefaultInstance())) - .build()); + Disease.Builder builder = Disease.newBuilder(); + term.ifPresent(builder::setTerm); + // v1 has no excluded field, hence no builder.setExcluded(...) + if (!stages.isEmpty()) builder.addAllDiseaseStage(stages); + if (!tnm.isEmpty()) builder.addAllClinicalTnmFinding(tnm); + onset.ifPresent(builder::setOnset); + + return Optional.of(builder.build()); } - static Optional toDiseaseOnset(org.phenopackets.schema.v1.core.Disease v1Disease) { + private static Optional toDiseaseOnset(org.phenopackets.schema.v1.core.Disease v1Disease) { if (v1Disease.hasClassOfOnset()) return OntologyClassConverter.toOntologyClass(v1Disease.getClassOfOnset()) .map(onsetClass -> TimeElement.newBuilder().setOntologyClass(onsetClass).build()); diff --git a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/EvidenceConverter.java b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/EvidenceConverter.java index 128b6f7e4..c944edb62 100644 --- a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/EvidenceConverter.java +++ b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/EvidenceConverter.java @@ -22,7 +22,7 @@ static List toEvidences(List .toList(); } - static Optional toEvidence(org.phenopackets.schema.v1.core.Evidence v1Evidence) { + private static Optional toEvidence(org.phenopackets.schema.v1.core.Evidence v1Evidence) { if (v1Evidence.equals(org.phenopackets.schema.v1.core.Evidence.getDefaultInstance())) return Optional.empty(); @@ -32,9 +32,9 @@ static Optional toEvidence(org.phenopackets.schema.v1.core.Evidence v1 if (evidenceCode.isEmpty() && externalReference.isEmpty()) return Optional.empty(); - return Optional.of(Evidence.newBuilder() - .setEvidenceCode(evidenceCode.orElse(OntologyClass.getDefaultInstance())) - .setReference(externalReference.orElse(ExternalReference.getDefaultInstance())) - .build()); + Evidence.Builder builder = Evidence.newBuilder(); + evidenceCode.ifPresent(builder::setEvidenceCode); + externalReference.ifPresent(builder::setReference); + return Optional.of(builder.build()); } } diff --git a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/PhenotypicFeatureConverter.java b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/PhenotypicFeatureConverter.java index 4f761a37c..731ee5b91 100644 --- a/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/PhenotypicFeatureConverter.java +++ b/phenopacket-tools-converter/src/main/java/org/phenopackets/phenopackettools/converter/converters/v2/PhenotypicFeatureConverter.java @@ -78,7 +78,7 @@ static Optional toPhenotypicFeature(org.phenopackets.schema.v } private static Optional toPhenotypicFeatureOnset(org.phenopackets.schema.v1.core.PhenotypicFeature v1PhenotypicFeature) { - if (v1PhenotypicFeature.equals(TimeElement.getDefaultInstance())) + if (v1PhenotypicFeature.equals(org.phenopackets.schema.v1.core.PhenotypicFeature.getDefaultInstance())) return Optional.empty(); boolean isDefault = true; diff --git a/phenopacket-tools-converter/src/test/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterTest.java b/phenopacket-tools-converter/src/test/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterTest.java index 5e6d8dc8c..fc1ee8a99 100644 --- a/phenopacket-tools-converter/src/test/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterTest.java +++ b/phenopacket-tools-converter/src/test/java/org/phenopackets/phenopackettools/converter/converters/V1ToV2ConverterTest.java @@ -1,6 +1,7 @@ package org.phenopackets.phenopackettools.converter.converters; import org.ga4gh.vrsatile.v1.Expression; +import org.ga4gh.vrsatile.v1.MoleculeContext; import org.ga4gh.vrsatile.v1.VariationDescriptor; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; @@ -101,6 +102,7 @@ public void convertPhenopacketWithConvertingVariants() { .setSyntax("hgvs") .setValue("NM_001848.2:c.877G>A") .build()) + .setMoleculeContext(MoleculeContext.transcript) .setAllelicState(OntologyClass.newBuilder() .setId("GENO:0000135") .setLabel("heterozygous") diff --git a/phenopacket-tools-core/pom.xml b/phenopacket-tools-core/pom.xml index 8555d7569..bd8859afa 100644 --- a/phenopacket-tools-core/pom.xml +++ b/phenopacket-tools-core/pom.xml @@ -6,7 +6,7 @@ phenopacket-tools org.phenopackets.phenopackettools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT phenopacket-tools-core diff --git a/phenopacket-tools-io/pom.xml b/phenopacket-tools-io/pom.xml index 089bf7959..f59ad0254 100644 --- a/phenopacket-tools-io/pom.xml +++ b/phenopacket-tools-io/pom.xml @@ -6,7 +6,7 @@ phenopacket-tools org.phenopackets.phenopackettools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT phenopacket-tools-io diff --git a/phenopacket-tools-test/pom.xml b/phenopacket-tools-test/pom.xml index 71ed5c16e..ed43f1a7f 100644 --- a/phenopacket-tools-test/pom.xml +++ b/phenopacket-tools-test/pom.xml @@ -5,7 +5,7 @@ phenopacket-tools org.phenopackets.phenopackettools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT 4.0.0 diff --git a/phenopacket-tools-util/pom.xml b/phenopacket-tools-util/pom.xml index 0db1f0956..51981b50c 100644 --- a/phenopacket-tools-util/pom.xml +++ b/phenopacket-tools-util/pom.xml @@ -5,7 +5,7 @@ phenopacket-tools org.phenopackets.phenopackettools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT 4.0.0 diff --git a/phenopacket-tools-util/src/main/java/module-info.java b/phenopacket-tools-util/src/main/java/module-info.java index b5ed35eac..8baadf63d 100644 --- a/phenopacket-tools-util/src/main/java/module-info.java +++ b/phenopacket-tools-util/src/main/java/module-info.java @@ -9,5 +9,6 @@ requires org.phenopackets.schema; exports org.phenopackets.phenopackettools.util.format; + exports org.phenopackets.phenopackettools.util.message; exports org.phenopackets.phenopackettools.util.print; } \ No newline at end of file diff --git a/phenopacket-tools-util/src/main/java/org/phenopackets/phenopackettools/util/message/MessageUtils.java b/phenopacket-tools-util/src/main/java/org/phenopackets/phenopackettools/util/message/MessageUtils.java new file mode 100644 index 000000000..137511d3f --- /dev/null +++ b/phenopacket-tools-util/src/main/java/org/phenopackets/phenopackettools/util/message/MessageUtils.java @@ -0,0 +1,66 @@ +package org.phenopackets.phenopackettools.util.message; + +import com.google.protobuf.MessageOrBuilder; + +import java.util.Collection; +import java.util.stream.Stream; + +/** + * A class with utility functions for working with protobuf messages. + */ +public class MessageUtils { + + private MessageUtils() { + // static utility class + } + + /** + * Find all instances of {@link T} in a given {@code message}. + *

+ * Protobuf messages are hierarchical structures where, depending on the schema, a component may be present + * at different places of the hierarchy. This function will stream all instances of the requested class. + * + * @param message protobuf message to search in + * @param clz target class + * @return a {@linkplain Stream} of all {@link T}s found in the {@code message} + * @param type of the target class + */ + @SuppressWarnings("unchecked") + public static Stream findInstancesOfType(MessageOrBuilder message, Class clz) { + Stream.Builder builder = Stream.builder(); + if (clz.isInstance(message)) { + /* + We suppress the warning regarding an unchecked cast of the message to T because we have checked + that `clz.isInstance(message)` holds. + */ + builder.add((T) message); + } + + for (Object field : message.getAllFields().values()) { + findAllInstances(field, clz, builder); + } + + return builder.build(); + } + + @SuppressWarnings("unchecked") + private static void findAllInstances(Object o, Class clz, Stream.Builder builder) { + if (clz.isInstance(o)) { + /* + We suppress the warning regarding an unchecked cast of `o` to T because we have checked + that `clz.isInstance(o)` holds. + */ + builder.add((U) o); + } else { + if (o instanceof MessageOrBuilder message) { + for (Object v : message.getAllFields().values()) { + findAllInstances(v, clz, builder); + } + } else if (o instanceof Collection collection) { + for (Object nestedField : collection) { + findAllInstances(nestedField, clz, builder); + } + } + } + } +} diff --git a/phenopacket-tools-util/src/main/java/org/phenopackets/phenopackettools/util/message/package-info.java b/phenopacket-tools-util/src/main/java/org/phenopackets/phenopackettools/util/message/package-info.java new file mode 100644 index 000000000..0d22cb63b --- /dev/null +++ b/phenopacket-tools-util/src/main/java/org/phenopackets/phenopackettools/util/message/package-info.java @@ -0,0 +1,4 @@ +/** + * Utility methods for working with protobuf messages. + */ +package org.phenopackets.phenopackettools.util.message; \ No newline at end of file diff --git a/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/TestResources.java b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/TestResources.java similarity index 79% rename from phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/TestResources.java rename to phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/TestResources.java index fc70e3e1d..24ef11339 100644 --- a/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/TestResources.java +++ b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/TestResources.java @@ -1,4 +1,4 @@ -package org.phenopackets.phenopackettools.util.format; +package org.phenopackets.phenopackettools.util; import java.nio.file.Path; diff --git a/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/ElementSnifferTest.java b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/ElementSnifferTest.java index 153d53bd7..8b82ab72f 100644 --- a/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/ElementSnifferTest.java +++ b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/ElementSnifferTest.java @@ -6,6 +6,7 @@ import org.phenopackets.phenopackettools.core.PhenopacketElement; import org.phenopackets.phenopackettools.core.PhenopacketFormat; import org.phenopackets.phenopackettools.core.PhenopacketSchemaVersion; +import org.phenopackets.phenopackettools.util.TestResources; import java.io.BufferedInputStream; import java.io.File; diff --git a/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/FormatSnifferTest.java b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/FormatSnifferTest.java index 1584a31d5..462aca79b 100644 --- a/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/FormatSnifferTest.java +++ b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/format/FormatSnifferTest.java @@ -3,6 +3,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.phenopackets.phenopackettools.core.PhenopacketFormat; +import org.phenopackets.phenopackettools.util.TestResources; import java.io.BufferedInputStream; import java.io.IOException; diff --git a/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/message/MessageUtilsTest.java b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/message/MessageUtilsTest.java new file mode 100644 index 000000000..a87bb64b7 --- /dev/null +++ b/phenopacket-tools-util/src/test/java/org/phenopackets/phenopackettools/util/message/MessageUtilsTest.java @@ -0,0 +1,67 @@ +package org.phenopackets.phenopackettools.util.message; + +import com.google.protobuf.Message; +import com.google.protobuf.util.JsonFormat; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.phenopackets.phenopackettools.util.TestResources; +import org.phenopackets.schema.v2.Phenopacket; +import org.phenopackets.schema.v2.core.Biosample; +import org.phenopackets.schema.v2.core.OntologyClass; +import org.phenopackets.schema.v2.core.PhenotypicFeature; +import org.phenopackets.schema.v2.core.Resource; + +import java.io.BufferedReader; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + +public class MessageUtilsTest { + + private static final Path TEST_DIR = TestResources.BASE_DIR.resolve("message"); + + + private static final Message.Builder COVID_PHENOPACKET = Phenopacket.newBuilder(); + + @BeforeAll + public static void beforeAll() throws IOException { + try (BufferedReader reader = Files.newBufferedReader(TEST_DIR.resolve("covid.json"))) { + JsonFormat.parser().merge(reader, COVID_PHENOPACKET); + } + } + + @Test + public void findAllOntologyClasses_PhenotypicFeatures() { + List pfs = MessageUtils.findInstancesOfType(COVID_PHENOPACKET, PhenotypicFeature.class) + .toList(); + assertThat(pfs, hasSize(8)); + List labels = pfs.stream() + .map(PhenotypicFeature::getType) + .map(OntologyClass::getLabel) + .toList(); + assertThat(labels, hasItems("Fever", "Flank pain", "Hematuria", "Stage 3 chronic kidney disease", "Myalgia", + "Diarrhea", "Dyspnea", "Acute respiratory distress syndrome")); + } + + @Test + public void findAllOntologyClasses_Resources() { + List resources = MessageUtils.findInstancesOfType(COVID_PHENOPACKET, Resource.class) + .toList(); + assertThat(resources, hasSize(2)); + List labels = resources.stream() + .map(Resource::getNamespacePrefix) + .toList(); + assertThat(labels, hasItems("NCIT", "MONDO")); + } + + @Test + public void findAllOntologyClasses_NoMatch() { + List resources = MessageUtils.findInstancesOfType(COVID_PHENOPACKET, Biosample.class) + .toList(); + assertThat(resources, is(emptyCollectionOf(Biosample.class))); + } +} \ No newline at end of file diff --git a/phenopacket-tools-util/src/test/resources/org/phenopackets/phenopackettools/util/format/covid.json b/phenopacket-tools-util/src/test/resources/org/phenopackets/phenopackettools/util/format/covid.json index 21125cb9d..52a8f2400 100644 --- a/phenopacket-tools-util/src/test/resources/org/phenopackets/phenopackettools/util/format/covid.json +++ b/phenopacket-tools-util/src/test/resources/org/phenopackets/phenopackettools/util/format/covid.json @@ -19,7 +19,7 @@ "phenotypicFeatures": [{ "type": { "id": "HP:0001945", - "label": "Fever " + "label": "Fever" }, "onset": { "timestamp": "2021-02-01T05:00:00Z" diff --git a/phenopacket-tools-util/src/test/resources/org/phenopackets/phenopackettools/util/message/covid.json b/phenopacket-tools-util/src/test/resources/org/phenopackets/phenopackettools/util/message/covid.json new file mode 100644 index 000000000..52a8f2400 --- /dev/null +++ b/phenopacket-tools-util/src/test/resources/org/phenopackets/phenopackettools/util/message/covid.json @@ -0,0 +1,317 @@ +{ + "id": "arbitrary.phenopacket.id", + "subject": { + "id": "P123542", + "timeAtLastEncounter": { + "age": { + "iso8601duration": "P70Y" + } + }, + "vitalStatus": { + "status": "DECEASED", + "causeOfDeath": { + "id": "MONDO:0100096", + "label": "COVID-19" + } + }, + "sex": "MALE" + }, + "phenotypicFeatures": [{ + "type": { + "id": "HP:0001945", + "label": "Fever" + }, + "onset": { + "timestamp": "2021-02-01T05:00:00Z" + } + }, { + "type": { + "id": "HP:0030157", + "label": "Flank pain" + }, + "onset": { + "timestamp": "2021-02-01T05:00:00Z" + } + }, { + "type": { + "id": "HP:0000790", + "label": "Hematuria" + }, + "onset": { + "timestamp": "2021-02-01T05:00:00Z" + } + }, { + "type": { + "id": "HP:0012625", + "label": "Stage 3 chronic kidney disease" + }, + "onset": { + "timestamp": "2021-02-01T05:00:00Z" + } + }, { + "type": { + "id": "HP:0003326", + "label": "Myalgia" + }, + "onset": { + "interval": { + "start": "2020-03-18T00:00:00Z", + "end": "2020-03-20T00:00:00Z" + } + } + }, { + "type": { + "id": "HP:0002014", + "label": "Diarrhea" + }, + "onset": { + "interval": { + "start": "2020-03-18T00:00:00Z", + "end": "2020-03-20T00:00:00Z" + } + } + }, { + "type": { + "id": "HP:0002094", + "label": "Dyspnea" + }, + "onset": { + "interval": { + "start": "2020-03-18T00:00:00Z", + "end": "2020-03-20T00:00:00Z" + } + } + }, { + "type": { + "id": "HP:0033677", + "label": "Acute respiratory distress syndrome" + }, + "onset": { + "timestamp": "2020-03-20T00:00:00Z" + } + }], + "measurements": [{ + "assay": { + "id": "LOINC:26474-7", + "label": "Lymphocytes [#/volume] in Blood" + }, + "value": { + "quantity": { + "unit": { + "id": "NCIT:C67245", + "label": "Thousand Cells" + }, + "value": 1.4 + } + }, + "timeObserved": { + "interval": { + "start": "2019-09-01T00:00:00Z", + "end": "2020-03-01T00:00:00Z" + } + } + }, { + "assay": { + "id": "LOINC:26474-7", + "label": "Lymphocytes [#/volume] in Blood" + }, + "value": { + "quantity": { + "unit": { + "id": "NCIT:C67245", + "label": "Thousand Cells" + }, + "value": 0.7 + } + }, + "timeObserved": { + "timestamp": "2020-03-20T00:00:00Z" + } + }], + "diseases": [{ + "term": { + "id": "MONDO:0005015", + "label": "diabetes mellitus" + }, + "excluded": true + }, { + "term": { + "id": "MONDO:0004994", + "label": "cardiomyopathy" + } + }, { + "term": { + "id": "MONDO:0100096", + "label": "COVID-19" + }, + "onset": { + "timestamp": "2020-03-17T00:00:00Z" + } + }], + "medicalActions": [{ + "procedure": { + "code": { + "id": "NCIT:C80473", + "label": "Left Ventricular Assist Device" + }, + "performed": { + "timestamp": "2016-01-01T00:00:00Z" + } + } + }, { + "treatment": { + "agent": { + "id": "NCIT:C722", + "label": "Oxygen" + }, + "routeOfAdministration": { + "id": "NCIT:C38284", + "label": "Nasal Route of Administration" + }, + "doseIntervals": [{ + "quantity": { + "unit": { + "id": "NCIT:C67388", + "label": "Liter per Minute" + }, + "value": 2.0 + }, + "scheduleFrequency": { + "id": "PATO:0000689", + "label": "continuous" + }, + "interval": { + "start": "2021-02-01T18:58:43Z", + "end": "2021-02-02T08:22:42Z" + } + }, { + "quantity": { + "unit": { + "id": "NCIT:C67388", + "label": "Liter per Minute" + }, + "value": 50.0 + }, + "scheduleFrequency": { + "id": "PATO:0000689", + "label": "continuous" + }, + "interval": { + "start": "2021-02-02T08:22:42Z", + "end": "2021-02-02T12:22:42Z" + } + }] + } + }, { + "treatment": { + "agent": { + "id": "CHEBI:41879", + "label": "dexamethasone" + }, + "doseIntervals": [{ + "quantity": { + "unit": { + "id": "UO:0000022", + "label": "milligram" + }, + "value": 6.0 + }, + "scheduleFrequency": { + "id": "NCIT:C125004", + "label": "Once Daily" + }, + "interval": { + "start": "2020-03-20T00:00:00Z", + "end": "2020-03-30T00:00:00Z" + } + }] + } + }, { + "procedure": { + "code": { + "id": "NCIT:C116648", + "label": "Tracheal Intubation" + }, + "performed": { + "timestamp": "2020-03-22T00:00:00Z" + } + } + }, { + "treatment": { + "agent": { + "id": "NCIT:C722", + "label": "Oxygen" + }, + "routeOfAdministration": { + "id": "NCIT:C50254", + "label": "Positive end Expiratory Pressure Valve Device" + }, + "doseIntervals": [{ + "quantity": { + "unit": { + "id": "NCIT:C91060", + "label": "Centimeters of Water" + }, + "value": 14.0 + }, + "scheduleFrequency": { + "id": "PATO:0000689", + "label": "continuous" + }, + "interval": { + "start": "2020-03-22T00:00:00Z", + "end": "2020-03-28T00:00:00Z" + } + }] + } + }, { + "treatment": { + "agent": { + "id": "NCIT:C84217", + "label": "Tocilizumab" + }, + "doseIntervals": [{ + "quantity": { + "unit": { + "id": "NCIT:C124458", + "label": "Milligram per Kilogram per Dose" + }, + "value": 4.0 + }, + "scheduleFrequency": { + "id": "NCIT:C64529", + "label": "Every Four Weeks" + }, + "interval": { + "start": "2020-03-24T00:00:00Z", + "end": "2020-03-28T00:00:00Z" + } + }] + } + }], + "metaData": { + "created": "2021-08-17T00:00:00Z", + "createdBy": "anonymous biocurator", + "resources": [{ + "id": "ncit", + "name": "NCI Thesaurus", + "url": "http://purl.obolibrary.org/obo/ncit.owl", + "version": "2019-11-26", + "namespacePrefix": "NCIT", + "iriPrefix": "http://purl.obolibrary.org/obo/NCIT_" + }, { + "id": "mondo", + "name": "Mondo Disease Ontology", + "url": "http://purl.obolibrary.org/obo/mondo.obo", + "version": "2021-11-26", + "namespacePrefix": "MONDO", + "iriPrefix": "http://purl.obolibrary.org/obo/MONDO_" + }], + "phenopacketSchemaVersion": "2.0", + "externalReferences": [{ + "id": "DOI:10.1016/j.jaccas.2020.04.001", + "reference": "PMID:32292915", + "description": "The Imperfect Cytokine Storm: Severe COVID-19 With ARDS in a Patient on Durable LVAD Support" + }] + } +} \ No newline at end of file diff --git a/phenopacket-tools-validator-core/pom.xml b/phenopacket-tools-validator-core/pom.xml index f9790dd66..8623068f2 100644 --- a/phenopacket-tools-validator-core/pom.xml +++ b/phenopacket-tools-validator-core/pom.xml @@ -7,7 +7,7 @@ org.phenopackets.phenopackettools phenopacket-tools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT phenopacket-tools-validator-core @@ -37,6 +37,11 @@ org.monarchinitiative.phenol phenol-core + + com.fasterxml.jackson.core + jackson-databind + provided + org.monarchinitiative.phenol diff --git a/phenopacket-tools-validator-core/src/main/java/module-info.java b/phenopacket-tools-validator-core/src/main/java/module-info.java index 75fe7dc85..8fbcfef45 100644 --- a/phenopacket-tools-validator-core/src/main/java/module-info.java +++ b/phenopacket-tools-validator-core/src/main/java/module-info.java @@ -14,6 +14,8 @@ requires org.monarchinitiative.phenol.core; requires org.phenopackets.schema; + requires com.fasterxml.jackson.databind; + // There are many places where the protobuf classes are part of the API, e.g. as type parameter // of PhenopacketFormatConverter. requires transitive com.google.protobuf; diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResult.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResult.java index f8193cd69..95de51490 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResult.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResult.java @@ -1,8 +1,14 @@ package org.phenopackets.phenopackettools.validator.core; +import com.fasterxml.jackson.annotation.JsonGetter; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + /** * {@code ValidationResult} contains results of a single validation step performed by a {@link PhenopacketValidator}. */ +@JsonSerialize(as = ValidationResult.class) +@JsonPropertyOrder({"validatorInfo", "level", "category", "message"}) public interface ValidationResult { /** @@ -36,21 +42,25 @@ static ValidationResult of(ValidatorInfo validatorInfo, /** * @return information about the validator used to create the {@link ValidationResult}. */ + @JsonGetter ValidatorInfo validatorInfo(); /** * @return level of the validation */ + @JsonGetter ValidationLevel level(); /** * @return an error category. */ + @JsonGetter String category(); /** * @return specific error message */ + @JsonGetter String message(); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResults.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResults.java index da4aa6334..4bb747a1b 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResults.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidationResults.java @@ -1,5 +1,10 @@ package org.phenopackets.phenopackettools.validator.core; +import com.fasterxml.jackson.annotation.JsonGetter; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + import java.util.ArrayList; import java.util.List; @@ -11,6 +16,8 @@ * The results contain info regarding which validators were run ({@link #validators()}) and the issues found during * the validation ({@link #validationResults()}). */ +@JsonSerialize(as = ValidationResults.class) +@JsonPropertyOrder({"validators", "validationResults"}) public interface ValidationResults { static ValidationResults of(List validators, @@ -31,16 +38,19 @@ static Builder builder() { /** * @return a list of {@link ValidatorInfo} representing validators applied to the top-level element. */ + @JsonGetter List validators(); /** * @return a list of {@link ValidationResult} representing the issues found in the top-level element. */ + @JsonGetter List validationResults(); /** * @return {@code true} if no issues have been found and the validated item is valid. */ + @JsonIgnore default boolean isValid() { return validationResults().isEmpty(); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidatorInfo.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidatorInfo.java index 3714b4ae6..50b1c5eb4 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidatorInfo.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/ValidatorInfo.java @@ -1,8 +1,14 @@ package org.phenopackets.phenopackettools.validator.core; +import com.fasterxml.jackson.annotation.JsonGetter; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + /** * A description of a {@link PhenopacketValidator}. */ +@JsonSerialize(as = ValidatorInfo.class) +@JsonPropertyOrder({"validatorId", "validatorName", "description"}) public interface ValidatorInfo { static ValidatorInfo baseSyntaxValidation() { @@ -25,16 +31,19 @@ static ValidatorInfo of(String validatorId, String validatorName, String descrip /** * @return string with a unique validator ID. */ + @JsonGetter String validatorId(); /** * @return human-friendly validator name. */ + @JsonGetter String validatorName(); /** * @return brief description of the validation provided by the validator. */ + @JsonGetter String description(); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/metadata/MetaDataValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/metadata/MetaDataValidator.java index 851e9673a..30dbea493 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/metadata/MetaDataValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/metadata/MetaDataValidator.java @@ -1,6 +1,7 @@ package org.phenopackets.phenopackettools.validator.core.metadata; import com.google.protobuf.MessageOrBuilder; +import org.phenopackets.phenopackettools.util.message.MessageUtils; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.ValidationResult; import org.phenopackets.phenopackettools.validator.core.ValidatorInfo; @@ -11,7 +12,6 @@ import org.phenopackets.schema.v2.core.OntologyClass; import org.phenopackets.schema.v2.core.Resource; -import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.Set; @@ -40,7 +40,7 @@ public List validate(T component) { Set validOntologyPrefixes = getOntologyNamespacePrefixes(metaData.get()); - return streamOfAllInstancesOfType(component, OntologyClass.class).sequential() + return MessageUtils.findInstancesOfType(component, OntologyClass.class).sequential() .flatMap(oc -> { // Curie should be something like `HP:1234567` or `NCIT_C123457`. String curie = oc.getId(); @@ -79,45 +79,6 @@ private static Set getOntologyNamespacePrefixes(MetaData metaData) { .collect(Collectors.toSet()); } - @SuppressWarnings("unchecked") - private static Stream streamOfAllInstancesOfType(MessageOrBuilder message, Class clz) { - Stream.Builder builder = Stream.builder(); - if (clz.isInstance(message)) { - /* - We suppress the warning regarding an unchecked cast of the message to T because we have checked - that `clz.isInstance(message)` holds. - */ - builder.add((U) message); - } - - for (Object field : message.getAllFields().values()) { - findAllInstances(field, clz, builder); - } - - return builder.build(); - } - - @SuppressWarnings("unchecked") - private static void findAllInstances(Object o, Class clz, Stream.Builder builder) { - if (clz.isInstance(o)) { - /* - We suppress the warning regarding an unchecked cast of `o` to T because we have checked - that `clz.isInstance(o)` holds. - */ - builder.add((U) o); - } else { - if (o instanceof MessageOrBuilder message) { - for (Object v : message.getAllFields().values()) { - findAllInstances(v, clz, builder); - } - } else if (o instanceof Collection collection) { - for (Object nestedField : collection) { - findAllInstances(nestedField, clz, builder); - } - } - } - } - static class PhenopacketMetaDataValidator extends MetaDataValidator { @Override diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/HpoPhenotypeValidators.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/HpoPhenotypeValidators.java index 40da8cb80..e23e1b03c 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/HpoPhenotypeValidators.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/HpoPhenotypeValidators.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.phenotype.ancestry.CohortHpoAncestryValidator; @@ -12,6 +12,7 @@ import org.phenopackets.phenopackettools.validator.core.phenotype.primary.CohortHpoPhenotypeValidator; import org.phenopackets.phenopackettools.validator.core.phenotype.primary.FamilyHpoPhenotypeValidator; import org.phenopackets.phenopackettools.validator.core.phenotype.primary.PhenopacketHpoPhenotypeValidator; +import org.phenopackets.phenopackettools.validator.core.phenotype.uniq.HpoUniqueValidator; import org.phenopackets.schema.v2.*; import java.util.Collection; @@ -26,39 +27,39 @@ private HpoPhenotypeValidators() { } /** - * Get {@link PhenopacketValidator} to validate {@link Phenopacket} using provided {@link Ontology}. + * Get {@link PhenopacketValidator} to validate {@link Phenopacket} using provided {@link MinimalOntology}. * * @param hpo HPO ontology - * @deprecated use {@link Primary#phenopacketHpoPhenotypeValidator(Ontology)} instead + * @deprecated use {@link Primary#phenopacketHpoPhenotypeValidator(MinimalOntology)} instead */ // REMOVE(v1.0.0) @Deprecated(forRemoval = true) - public static PhenopacketValidator phenopacketHpoPhenotypeValidator(Ontology hpo) { + public static PhenopacketValidator phenopacketHpoPhenotypeValidator(MinimalOntology hpo) { return Primary.phenopacketHpoPhenotypeValidator(hpo); } /** - * Get {@link PhenopacketValidator} for validate {@link Family} using provided {@link Ontology}. + * Get {@link PhenopacketValidator} for validate {@link Family} using provided {@link MinimalOntology}. * * @param hpo HPO ontology - * @deprecated use {@link Primary#familyHpoPhenotypeValidator(Ontology)} instead + * @deprecated use {@link Primary#familyHpoPhenotypeValidator(MinimalOntology)} instead */ // REMOVE(v1.0.0) @Deprecated(forRemoval = true) - public static PhenopacketValidator familyHpoPhenotypeValidator(Ontology hpo) { + public static PhenopacketValidator familyHpoPhenotypeValidator(MinimalOntology hpo) { return Primary.familyHpoPhenotypeValidator(hpo); } /** - * Get {@link PhenopacketValidator} for performing primary validation {@link Cohort} using provided {@link Ontology}, + * Get {@link PhenopacketValidator} for performing primary validation {@link Cohort} using provided {@link MinimalOntology}, * as described in {@link org.phenopackets.phenopackettools.validator.core.phenotype.primary.AbstractHpoPhenotypeValidator}. * * @param hpo HPO ontology - * @deprecated use {@link Primary#cohortHpoPhenotypeValidator(Ontology)} instead + * @deprecated use {@link Primary#cohortHpoPhenotypeValidator(MinimalOntology)} instead */ // REMOVE(v1.0.0) @Deprecated(forRemoval = true) - public static PhenopacketValidator cohortHpoPhenotypeValidator(Ontology hpo) { + public static PhenopacketValidator cohortHpoPhenotypeValidator(MinimalOntology hpo) { return Primary.cohortHpoPhenotypeValidator(hpo); } @@ -69,30 +70,30 @@ public static PhenopacketValidator cohortHpoPhenotypeValidator( */ public static class Primary { /** - * Get {@link PhenopacketValidator} to validate {@link Phenopacket} using provided {@link Ontology}. + * Get {@link PhenopacketValidator} to validate {@link Phenopacket} using provided {@link MinimalOntology}. * * @param hpo HPO ontology */ - public static PhenopacketValidator phenopacketHpoPhenotypeValidator(Ontology hpo) { + public static PhenopacketValidator phenopacketHpoPhenotypeValidator(MinimalOntology hpo) { return new PhenopacketHpoPhenotypeValidator(hpo); } /** - * Get {@link PhenopacketValidator} for validate {@link Family} using provided {@link Ontology}. + * Get {@link PhenopacketValidator} for validate {@link Family} using provided {@link MinimalOntology}. * * @param hpo HPO ontology */ - public static PhenopacketValidator familyHpoPhenotypeValidator(Ontology hpo) { + public static PhenopacketValidator familyHpoPhenotypeValidator(MinimalOntology hpo) { return new FamilyHpoPhenotypeValidator(hpo); } /** - * Get {@link PhenopacketValidator} for performing primary validation {@link Cohort} using provided {@link Ontology}, + * Get {@link PhenopacketValidator} for performing primary validation {@link Cohort} using provided {@link MinimalOntology}, * as described in {@link org.phenopackets.phenopackettools.validator.core.phenotype.primary.AbstractHpoPhenotypeValidator}. * * @param hpo HPO ontology */ - public static PhenopacketValidator cohortHpoPhenotypeValidator(Ontology hpo) { + public static PhenopacketValidator cohortHpoPhenotypeValidator(MinimalOntology hpo) { return new CohortHpoPhenotypeValidator(hpo); } } @@ -115,29 +116,29 @@ private Ancestry() { } /** - * Get {@link PhenopacketValidator} to validate ancestry {@link Phenopacket} using provided {@link Ontology}. + * Get {@link PhenopacketValidator} to validate ancestry {@link Phenopacket} using provided {@link MinimalOntology}. * * @param hpo HPO ontology */ - public static PhenopacketValidator phenopacketHpoAncestryValidator(Ontology hpo) { + public static PhenopacketValidator phenopacketHpoAncestryValidator(MinimalOntology hpo) { return new PhenopacketHpoAncestryValidator(hpo); } /** - * Get {@link PhenopacketValidator} to validate ancestry {@link Family} using provided {@link Ontology}. + * Get {@link PhenopacketValidator} to validate ancestry {@link Family} using provided {@link MinimalOntology}. * * @param hpo HPO ontology */ - public static PhenopacketValidator familyHpoAncestryValidator(Ontology hpo) { + public static PhenopacketValidator familyHpoAncestryValidator(MinimalOntology hpo) { return new FamilyHpoAncestryValidator(hpo); } /** - * Get {@link PhenopacketValidator} to validate ancestry {@link Cohort} using provided {@link Ontology}. + * Get {@link PhenopacketValidator} to validate ancestry {@link Cohort} using provided {@link MinimalOntology}. * * @param hpo HPO ontology */ - public static PhenopacketValidator cohortHpoAncestryValidator(Ontology hpo) { + public static PhenopacketValidator cohortHpoAncestryValidator(MinimalOntology hpo) { return new CohortHpoAncestryValidator(hpo); } } @@ -160,48 +161,84 @@ private OrganSystem() { /** * Get {@link PhenopacketValidator} to validate annotation of organ systems in a {@link Phenopacket} - * using provided {@link Ontology} and a collection of organ system {@link TermId}s. + * using provided {@link MinimalOntology} and a collection of organ system {@link TermId}s. *

- * NOTE: the organ system {@link TermId} that is absent from the {@link Ontology} is disregarded + * NOTE: the organ system {@link TermId} that is absent from the {@link MinimalOntology} is disregarded * and not used for validation. * * @param hpo HPO ontology * @param organSystemTermIds a collection of HPO {@link TermId}s corresponding to organ systems. */ - public static PhenopacketValidator phenopacketHpoOrganSystemValidator(Ontology hpo, + public static PhenopacketValidator phenopacketHpoOrganSystemValidator(MinimalOntology hpo, Collection organSystemTermIds) { return new PhenopacketHpoOrganSystemValidator(hpo, organSystemTermIds); } /** * Get {@link PhenopacketValidator} to validate annotation of organ systems in a {@link Family} - * using provided {@link Ontology} and a collection of organ system {@link TermId}s. + * using provided {@link MinimalOntology} and a collection of organ system {@link TermId}s. *

- * NOTE: the organ system {@link TermId} that is absent from the {@link Ontology} is disregarded + * NOTE: the organ system {@link TermId} that is absent from the {@link MinimalOntology} is disregarded * and not used for validation. * * @param hpo HPO ontology * @param organSystemTermIds a collection of HPO {@link TermId}s corresponding to organ systems. */ - public static PhenopacketValidator familyHpoOrganSystemValidator(Ontology hpo, + public static PhenopacketValidator familyHpoOrganSystemValidator(MinimalOntology hpo, Collection organSystemTermIds) { return new FamilyHpoOrganSystemValidator(hpo, organSystemTermIds); } /** * Get {@link PhenopacketValidator} to validate annotation of organ systems in a {@link Cohort} - * using provided {@link Ontology} and a collection of organ system {@link TermId}s. + * using provided {@link MinimalOntology} and a collection of organ system {@link TermId}s. *

- * NOTE: the organ system {@link TermId} that is absent from the {@link Ontology} is disregarded + * NOTE: the organ system {@link TermId} that is absent from the {@link MinimalOntology} is disregarded * and not used for validation. * * @param hpo HPO ontology * @param organSystemTermIds a collection of HPO {@link TermId}s corresponding to organ systems. */ - public static PhenopacketValidator cohortHpoOrganSystemValidator(Ontology hpo, + public static PhenopacketValidator cohortHpoOrganSystemValidator(MinimalOntology hpo, Collection organSystemTermIds) { return new CohortHpoOrganSystemValidator(hpo, organSystemTermIds); } } + /** + * A static factory class for providing {@link org.phenopackets.phenopackettools.validator.core.PhenopacketValidator}s + * that check if HPO terms of the Phenopacket schema elements are unique - specified only once. + */ + public static class Unique { + private Unique() { + } + + /** + * Get {@link PhenopacketValidator} to validate {@link Phenopacket} using provided ontology. + * + * @param hpo HPO ontology + */ + public static PhenopacketValidator phenopacketValidator(MinimalOntology hpo) { + return HpoUniqueValidator.phenopacketValidator(hpo); + } + + /** + * Get {@link PhenopacketValidator} to validate {@link Cohort} using provided ontology. + * + * @param hpo HPO ontology + */ + public static PhenopacketValidator cohortValidator(MinimalOntology hpo) { + return HpoUniqueValidator.cohortValidator(hpo); + } + + /** + * Get {@link PhenopacketValidator} to validate {@link Family} using provided ontology. + * + * @param hpo HPO ontology + */ + public static PhenopacketValidator familyValidator(MinimalOntology hpo) { + return HpoUniqueValidator.familyValidator(hpo); + } + } + } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/AbstractHpoAncestryValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/AbstractHpoAncestryValidator.java index 4da56932e..a074a2e40 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/AbstractHpoAncestryValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/AbstractHpoAncestryValidator.java @@ -1,8 +1,7 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.ancestry; import com.google.protobuf.MessageOrBuilder; -import org.monarchinitiative.phenol.ontology.algo.OntologyAlgorithm; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.Term; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.phenopackettools.validator.core.ValidationResult; @@ -16,6 +15,7 @@ import org.slf4j.LoggerFactory; import java.util.*; +import java.util.stream.Collectors; import java.util.stream.Stream; /** @@ -39,10 +39,13 @@ public abstract class AbstractHpoAncestryValidator e "HPO ancestry phenotypic feature validator", "Validate that phenopacket does not contain an HPO term and its ancestor based on the provided HPO"); private static final String APR_VIOLATION = "Violation of the annotation propagation rule"; - private static final String UNKNOWN = "UNKNOWN_NAME"; - AbstractHpoAncestryValidator(Ontology hpo) { + private final Set obsoleteTermIds; + + AbstractHpoAncestryValidator(MinimalOntology hpo) { super(hpo); + this.obsoleteTermIds = hpo.obsoleteTermIdsStream() + .collect(Collectors.toSet()); } @Override @@ -67,12 +70,12 @@ private Stream validatePhenopacketPhenotypicFeatures(String id // Check that the component does not contain both observed term and its ancestor. for (TermId observed : featuresByExclusion.observedPhenotypicFeatures()) { - if (isObsoleteTermId(observed)) { + if (obsoleteTermIds.contains(observed)) { LOGGER.debug("Ignoring unknown/obsolete term ID {}", observed.getValue()); continue; } - for (TermId ancestor : OntologyAlgorithm.getAncestorTerms(hpo, observed, false)) { + for (TermId ancestor : hpo.graph().getAncestors(observed, false)) { if (featuresByExclusion.observedPhenotypicFeatures().contains(ancestor)) results.add(constructResultForAnObservedTerm(id, observed, ancestor, false)); if (featuresByExclusion.excludedPhenotypicFeatures().contains(ancestor)) @@ -82,15 +85,12 @@ private Stream validatePhenopacketPhenotypicFeatures(String id // Check that the component does not have negated descendant for (TermId excluded : featuresByExclusion.excludedPhenotypicFeatures()) { - if (isObsoleteTermId(excluded)) { + if (obsoleteTermIds.contains(excluded)) { LOGGER.debug("Ignoring unknown/obsolete term ID {}", excluded.getValue()); continue; } - for (TermId child : OntologyAlgorithm.getDescendents(hpo, excluded)) { - if (child.equals(excluded)) - // skip the parent term - continue; + for (TermId child : hpo.graph().getDescendants(excluded, false)) { if (featuresByExclusion.excludedPhenotypicFeatures().contains(child)) results.add(constructResultForAnExcludedTerm(id, excluded, child)); } @@ -99,15 +99,13 @@ private Stream validatePhenopacketPhenotypicFeatures(String id return results.build(); } - private boolean isObsoleteTermId(TermId termId) { - return hpo.getObsoleteTermIds().contains(termId); - } - private ValidationResult constructResultForAnObservedTerm(String id, TermId observedId, TermId ancestorId, boolean ancestorIsExcluded) { - Term observedTerm = hpo.getTermMap().get(observedId); - String observedTermName = observedTerm == null ? UNKNOWN : observedTerm.getName(); - Term ancestorTerm = hpo.getTermMap().get(ancestorId); - String ancestorTermName = ancestorTerm == null ? UNKNOWN : ancestorTerm.getName(); + String observedTermName = hpo.termForTermId(observedId) + .map(Term::getName) + .orElse(UNKNOWN); + String ancestorTermName = hpo.termForTermId(ancestorId) + .map(Term::getName) + .orElse(UNKNOWN); String message; if (ancestorIsExcluded) message = "Phenotypic features of %s must not contain both an observed term (%s, %s) and an excluded ancestor (%s, %s)".formatted( @@ -120,10 +118,12 @@ private ValidationResult constructResultForAnObservedTerm(String id, TermId obse } private ValidationResult constructResultForAnExcludedTerm(String id, TermId excluded, TermId child) { - Term excludedTerm = hpo.getTermMap().get(excluded); - String excludedTermName = excludedTerm == null ? UNKNOWN : excludedTerm.getName(); - Term childTerm = hpo.getTermMap().get(child); - String childTermName = childTerm == null ? UNKNOWN : childTerm.getName(); + String excludedTermName = hpo.termForTermId(excluded) + .map(Term::getName) + .orElse(UNKNOWN); + String childTermName = hpo.termForTermId(child) + .map(Term::getName) + .orElse(UNKNOWN); String message = "Phenotypic features of %s must not contain both an excluded term (%s, %s) and an excluded child (%s, %s)".formatted( id, excludedTermName, excluded.getValue(), childTermName, child.getValue()); diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/CohortHpoAncestryValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/CohortHpoAncestryValidator.java index c68a35175..1f1d9bbc8 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/CohortHpoAncestryValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/CohortHpoAncestryValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.ancestry; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.schema.v2.CohortOrBuilder; import org.phenopackets.schema.v2.PhenopacketOrBuilder; @@ -8,7 +8,7 @@ public class CohortHpoAncestryValidator extends AbstractHpoAncestryValidator { - public CohortHpoAncestryValidator(Ontology hpo) { + public CohortHpoAncestryValidator(MinimalOntology hpo) { super(hpo); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/FamilyHpoAncestryValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/FamilyHpoAncestryValidator.java index 66ba7e88b..57a9d3848 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/FamilyHpoAncestryValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/FamilyHpoAncestryValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.ancestry; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.schema.v2.FamilyOrBuilder; import org.phenopackets.schema.v2.Phenopacket; import org.phenopackets.schema.v2.PhenopacketOrBuilder; @@ -9,7 +9,7 @@ public class FamilyHpoAncestryValidator extends AbstractHpoAncestryValidator { - public FamilyHpoAncestryValidator(Ontology hpo) { + public FamilyHpoAncestryValidator(MinimalOntology hpo) { super(hpo); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/PhenopacketHpoAncestryValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/PhenopacketHpoAncestryValidator.java index b23ca49a9..3e87d72a1 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/PhenopacketHpoAncestryValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/ancestry/PhenopacketHpoAncestryValidator.java @@ -1,13 +1,13 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.ancestry; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.schema.v2.PhenopacketOrBuilder; import java.util.stream.Stream; public class PhenopacketHpoAncestryValidator extends AbstractHpoAncestryValidator { - public PhenopacketHpoAncestryValidator(Ontology hpo) { + public PhenopacketHpoAncestryValidator(MinimalOntology hpo) { super(hpo); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/base/BaseHpoValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/base/BaseHpoValidator.java index 529569f4e..321df50a8 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/base/BaseHpoValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/base/BaseHpoValidator.java @@ -1,7 +1,7 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.base; import com.google.protobuf.MessageOrBuilder; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.schema.v2.PhenopacketOrBuilder; @@ -9,10 +9,12 @@ public abstract class BaseHpoValidator implements PhenopacketValidator { - protected final Ontology hpo; + protected static final String UNKNOWN = "UNKNOWN_NAME"; + + protected final MinimalOntology hpo; protected final String hpoVersion; - protected BaseHpoValidator(Ontology hpo) { + protected BaseHpoValidator(MinimalOntology hpo) { this.hpo = Objects.requireNonNull(hpo); this.hpoVersion = hpo.version().orElse("UNKNOWN"); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/AbstractOrganSystemValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/AbstractOrganSystemValidator.java index 2c1fff7c9..020fa6bfb 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/AbstractOrganSystemValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/AbstractOrganSystemValidator.java @@ -1,8 +1,7 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.orgsys; import com.google.protobuf.MessageOrBuilder; -import org.monarchinitiative.phenol.ontology.algo.OntologyAlgorithm; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.Term; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.phenopackettools.validator.core.ValidationResult; @@ -41,7 +40,7 @@ public abstract class AbstractOrganSystemValidator e protected final List organSystemTermIds; - protected AbstractOrganSystemValidator(Ontology hpo, + protected AbstractOrganSystemValidator(MinimalOntology hpo, Collection organSystemTermIds) { super(hpo); this.organSystemTermIds = Objects.requireNonNull(organSystemTermIds).stream() @@ -51,9 +50,9 @@ protected AbstractOrganSystemValidator(Ontology hpo, .toList(); } - private static Predicate organSystemTermIdIsInOntology(Ontology hpo) { + private static Predicate organSystemTermIdIsInOntology(MinimalOntology hpo) { return organSystemTermId -> { - if (hpo.containsTerm(organSystemTermId)) { + if (hpo.termForTermId(organSystemTermId).isPresent()) { return true; } else { LOGGER.warn("{} is not present in the ontology", organSystemTermId.getValue()); @@ -90,13 +89,16 @@ private Stream checkPhenotypicFeatures(PhenopacketOrBuilder ph // Check if we have at least one observed annotation for the organ system. for (TermId pf : featuresByExclusion.observedPhenotypicFeatures()) { - if (OntologyAlgorithm.existsPath(hpo, pf, organSystemId)) { + if (hpo.graph().existsPath(pf, organSystemId)) continue organSystemLoop; // It only takes one termId to annotate an organ system. - } } + // We know that the organSystemId is in the HPO because we check the organ system ID presence + // in the constructor. + //noinspection OptionalGetWithoutIsPresent + Term organSystem = hpo.termForTermId(organSystemId).get(); + // The organSystemId is neither annotated nor excluded. We report a validation error. - Term organSystem = hpo.getTermMap().get(organSystemId); ValidationResult result = ValidationResult.error(VALIDATOR_INFO, MISSING_ORGAN_SYSTEM_CATEGORY, "Missing annotation for %s [%s]%s" diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/CohortHpoOrganSystemValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/CohortHpoOrganSystemValidator.java index 96bea28ba..8eabd7bfb 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/CohortHpoOrganSystemValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/CohortHpoOrganSystemValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.orgsys; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.schema.v2.CohortOrBuilder; import org.phenopackets.schema.v2.PhenopacketOrBuilder; @@ -10,7 +10,7 @@ public class CohortHpoOrganSystemValidator extends AbstractOrganSystemValidator { - public CohortHpoOrganSystemValidator(Ontology hpo, Collection organSystemTermIds) { + public CohortHpoOrganSystemValidator(MinimalOntology hpo, Collection organSystemTermIds) { super(hpo, organSystemTermIds); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/FamilyHpoOrganSystemValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/FamilyHpoOrganSystemValidator.java index 7edf20dfc..dddae345c 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/FamilyHpoOrganSystemValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/FamilyHpoOrganSystemValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.orgsys; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.schema.v2.FamilyOrBuilder; import org.phenopackets.schema.v2.PhenopacketOrBuilder; @@ -10,7 +10,7 @@ public class FamilyHpoOrganSystemValidator extends AbstractOrganSystemValidator { - public FamilyHpoOrganSystemValidator(Ontology hpo, Collection organSystemTermIds) { + public FamilyHpoOrganSystemValidator(MinimalOntology hpo, Collection organSystemTermIds) { super(hpo, organSystemTermIds); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/PhenopacketHpoOrganSystemValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/PhenopacketHpoOrganSystemValidator.java index d4db9e81d..67ae16ca3 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/PhenopacketHpoOrganSystemValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/orgsys/PhenopacketHpoOrganSystemValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.orgsys; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.schema.v2.PhenopacketOrBuilder; @@ -9,7 +9,7 @@ public class PhenopacketHpoOrganSystemValidator extends AbstractOrganSystemValidator { - public PhenopacketHpoOrganSystemValidator(Ontology hpo, + public PhenopacketHpoOrganSystemValidator(MinimalOntology hpo, Collection organSystemTerms) { super(hpo, organSystemTerms); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/AbstractHpoPhenotypeValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/AbstractHpoPhenotypeValidator.java index d75cfc7fb..95ce0f39a 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/AbstractHpoPhenotypeValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/AbstractHpoPhenotypeValidator.java @@ -2,13 +2,15 @@ import com.google.protobuf.MessageOrBuilder; import org.monarchinitiative.phenol.base.PhenolRuntimeException; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; +import org.monarchinitiative.phenol.ontology.data.Term; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.phenopackettools.validator.core.*; import org.phenopackets.phenopackettools.validator.core.phenotype.base.BaseHpoValidator; import org.phenopackets.schema.v2.PhenopacketOrBuilder; import org.phenopackets.schema.v2.core.PhenotypicFeature; +import java.util.Optional; import java.util.stream.Stream; public abstract class AbstractHpoPhenotypeValidator extends BaseHpoValidator { @@ -20,7 +22,7 @@ public abstract class AbstractHpoPhenotypeValidator private static final String INVALID_TERM_ID = "Invalid TermId"; private static final String OBSOLETED_TERM_ID = "Obsoleted TermId"; - public AbstractHpoPhenotypeValidator(Ontology hpo) { + public AbstractHpoPhenotypeValidator(MinimalOntology hpo) { super(hpo); } @@ -43,7 +45,8 @@ protected Stream checkPhenotypeFeature(PhenopacketOr } if (termId.getPrefix().equals("HP")) { // Check if the HPO contains the term. - if (!hpo.containsTerm(termId)) { + Optional term = hpo.termForTermId(termId); + if (term.isEmpty()) { String idSummary = summarizePhenopacketAndIndividualId(phenopacket); String msg = "%s%s not found in %s".formatted(termId.getValue(), idSummary, hpoVersion); return Stream.of( @@ -52,7 +55,7 @@ protected Stream checkPhenotypeFeature(PhenopacketOr } // Check if the `termId` is a primary ID. // If not, this is a warning. - TermId primaryId = hpo.getPrimaryTermId(termId); + TermId primaryId = term.get().id(); if (!primaryId.equals(termId)) { String idSummary = summarizePhenopacketAndIndividualId(phenopacket); String msg = "Using obsolete id (%s) instead of current primary id (%s)%s".formatted( diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/CohortHpoPhenotypeValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/CohortHpoPhenotypeValidator.java index 0642f21e2..b7beff122 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/CohortHpoPhenotypeValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/CohortHpoPhenotypeValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.primary; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.ValidationResult; import org.phenopackets.schema.v2.CohortOrBuilder; import org.phenopackets.schema.v2.Phenopacket; @@ -11,7 +11,7 @@ public class CohortHpoPhenotypeValidator extends AbstractHpoPhenotypeValidator { - public CohortHpoPhenotypeValidator(Ontology hpo) { + public CohortHpoPhenotypeValidator(MinimalOntology hpo) { super(hpo); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/FamilyHpoPhenotypeValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/FamilyHpoPhenotypeValidator.java index 65beef4e4..179754fba 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/FamilyHpoPhenotypeValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/FamilyHpoPhenotypeValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.primary; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.ValidationResult; import org.phenopackets.schema.v2.FamilyOrBuilder; import org.phenopackets.schema.v2.Phenopacket; @@ -11,7 +11,7 @@ public class FamilyHpoPhenotypeValidator extends AbstractHpoPhenotypeValidator { - public FamilyHpoPhenotypeValidator(Ontology hpo) { + public FamilyHpoPhenotypeValidator(MinimalOntology hpo) { super(hpo); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/PhenopacketHpoPhenotypeValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/PhenopacketHpoPhenotypeValidator.java index 7b43a10e7..dcd60bf63 100644 --- a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/PhenopacketHpoPhenotypeValidator.java +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/primary/PhenopacketHpoPhenotypeValidator.java @@ -1,6 +1,6 @@ package org.phenopackets.phenopackettools.validator.core.phenotype.primary; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.ValidationResult; import org.phenopackets.schema.v2.PhenopacketOrBuilder; import org.phenopackets.schema.v2.core.PhenotypicFeature; @@ -10,7 +10,7 @@ public class PhenopacketHpoPhenotypeValidator extends AbstractHpoPhenotypeValidator { - public PhenopacketHpoPhenotypeValidator(Ontology hpo) { + public PhenopacketHpoPhenotypeValidator(MinimalOntology hpo) { super(hpo); } diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/uniq/HpoUniqueValidator.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/uniq/HpoUniqueValidator.java new file mode 100644 index 000000000..895ec1b39 --- /dev/null +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/uniq/HpoUniqueValidator.java @@ -0,0 +1,159 @@ +package org.phenopackets.phenopackettools.validator.core.phenotype.uniq; + +import com.google.protobuf.MessageOrBuilder; +import org.monarchinitiative.phenol.base.PhenolRuntimeException; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; +import org.monarchinitiative.phenol.ontology.data.Term; +import org.monarchinitiative.phenol.ontology.data.TermId; +import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; +import org.phenopackets.phenopackettools.validator.core.ValidationResult; +import org.phenopackets.phenopackettools.validator.core.ValidatorInfo; +import org.phenopackets.phenopackettools.validator.core.phenotype.base.BaseHpoValidator; +import org.phenopackets.schema.v2.CohortOrBuilder; +import org.phenopackets.schema.v2.FamilyOrBuilder; +import org.phenopackets.schema.v2.Phenopacket; +import org.phenopackets.schema.v2.PhenopacketOrBuilder; +import org.phenopackets.schema.v2.core.PhenotypicFeature; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +/** + * Check that phenotypic features of a phenopacket are present at most once. + *

+ * The validator groups the phenotypic features by the observation status + * and tests that the phenotypic features are present at most once. + */ +public abstract class HpoUniqueValidator extends BaseHpoValidator { + + private static final Logger LOGGER = LoggerFactory.getLogger(HpoUniqueValidator.class); + + private static final ValidatorInfo VALIDATOR_INFO = ValidatorInfo.of( + "HpoUniqueValidator", + "HPO unique phenotypic feature validator", + "Validate that phenopacket does not contain an HPO term more than once"); + private static final String CATEGORY = "Non-unique phenotypic feature"; + + public static PhenopacketValidator phenopacketValidator(MinimalOntology hpo) { + return new PhenopacketHpoUniqueValidator(hpo); + } + + public static PhenopacketValidator cohortValidator(MinimalOntology hpo) { + return new CohortHpoUniqueValidator(hpo); + } + + public static PhenopacketValidator familyValidator(MinimalOntology hpo) { + return new FamilyHpoUniqueValidator(hpo); + } + + private HpoUniqueValidator(MinimalOntology hpo) { + super(hpo); + } + + @Override + public ValidatorInfo validatorInfo() { + return VALIDATOR_INFO; + } + + @Override + public List validate(T component) { + return extractPhenopackets(component) + .flatMap(pp -> validatePhenotypicFeatures(pp.getId(), pp.getPhenotypicFeaturesList())) + .toList(); + } + + protected abstract Stream extractPhenopackets(T message); + + private Stream validatePhenotypicFeatures(String id, Iterable phenotypicFeatures) { + // Count feature occurrences + Map> counter = new HashMap<>(); + for (PhenotypicFeature pf : phenotypicFeatures) { + counter.computeIfAbsent(pf.getExcluded(), s -> new HashMap<>()) + .merge(pf.getType().getId(), 1, Integer::sum); + } + + // Build results, first for the present features then for the excluded. + Stream.Builder results = Stream.builder(); + for (Map.Entry e : counter.getOrDefault(false, Map.of()).entrySet()) { + if (e.getValue() > 1) { + String termName = extractTermName(e.getKey()); + String msg = """ + Phenotypic features of %s must not contain the same observed feature %s (%s) + more than once but the feature was present %d times""".formatted(id, + termName, e.getKey(), e.getValue()); + results.add(ValidationResult.error(VALIDATOR_INFO, CATEGORY, msg)); + } + } + + for (Map.Entry e : counter.getOrDefault(true, Map.of()).entrySet()) { + if (e.getValue() > 1) { + String termName = extractTermName(e.getKey()); + String msg = """ + Phenotypic features of %s must not contain the same excluded feature %s (%s) + more than once but the feature was present %d times""".formatted(id, + termName, e.getKey(), e.getValue()); + results.add(ValidationResult.error(VALIDATOR_INFO, CATEGORY, msg)); + } + + } + + return results.build(); + } + + private String extractTermName(String curie) { + try { + return hpo.termForTermId(TermId.of(curie)) + .map(Term::getName) + .orElse(UNKNOWN); + } catch (PhenolRuntimeException ex) { + LOGGER.debug("Invalid term ID {}", curie, ex); + return UNKNOWN; + } + } + + private static class PhenopacketHpoUniqueValidator extends HpoUniqueValidator { + + private PhenopacketHpoUniqueValidator(MinimalOntology hpo) { + super(hpo); + } + + @Override + protected Stream extractPhenopackets(PhenopacketOrBuilder message) { + return Stream.of(message); + } + } + + private static class CohortHpoUniqueValidator extends HpoUniqueValidator { + + public CohortHpoUniqueValidator(MinimalOntology hpo) { + super(hpo); + } + + @Override + protected Stream extractPhenopackets(CohortOrBuilder message) { + return message.getMembersList().stream(); + } + } + + private static class FamilyHpoUniqueValidator extends HpoUniqueValidator { + + public FamilyHpoUniqueValidator(MinimalOntology hpo) { + super(hpo); + } + + @Override + protected Stream extractPhenopackets(FamilyOrBuilder message) { + Stream.Builder builder = Stream.builder(); + builder.accept(message.getProband()); + + for (Phenopacket relative : message.getRelativesList()) + builder.add(relative); + + return builder.build(); + } + } +} diff --git a/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/uniq/package-info.java b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/uniq/package-info.java new file mode 100644 index 000000000..546f6aa87 --- /dev/null +++ b/phenopacket-tools-validator-core/src/main/java/org/phenopackets/phenopackettools/validator/core/phenotype/uniq/package-info.java @@ -0,0 +1,10 @@ +/** + * The {@code uniq} package checks uniqueness of the phenotypic features - each feature must be specified exactly once. + *

+ * Currently, the validators only perform naive check, largely ignoring the phenotypic feature + * modifiers, except of the {@linkplain org.phenopackets.schema.v2.core.PhenotypicFeature#getExcluded()} field. + * A phenotypic feature can only be present once within the present or excluded features. + * Note, however, that other validators must check the extreme case of a feature being present and excluded + * at the same time. + */ +package org.phenopackets.phenopackettools.validator.core.phenotype.uniq; \ No newline at end of file diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/TestData.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/TestData.java index f26901d32..58f6e3a2e 100644 --- a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/TestData.java +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/TestData.java @@ -1,15 +1,27 @@ package org.phenopackets.phenopackettools.validator.core; -import org.monarchinitiative.phenol.io.OntologyLoader; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.io.MinimalOntologyLoader; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; -import java.nio.file.Path; +import java.io.BufferedInputStream; +import java.io.IOException; +import java.util.Objects; +import java.util.zip.GZIPInputStream; public class TestData { - public static final Path TEST_BASE_DIR = Path.of("src/test/resources/org/phenopackets/phenopackettools/validator/core"); - private static final Path HPO_MODULE_PATH = TEST_BASE_DIR.resolve("hp.module.json"); + public static final MinimalOntology HPO; - public static final Ontology HPO = OntologyLoader.loadOntology(HPO_MODULE_PATH.toFile()); + static { + try { + HPO = MinimalOntologyLoader.loadOntology(new GZIPInputStream( + new BufferedInputStream( + Objects.requireNonNull(TestData.class.getResourceAsStream("hp.v2026-01-08.json.gz")) + ) + )); + } catch (IOException e) { + throw new RuntimeException(e); + } + } } diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidationResultTest.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidationResultTest.java new file mode 100644 index 000000000..59ebd75e3 --- /dev/null +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidationResultTest.java @@ -0,0 +1,34 @@ +package org.phenopackets.phenopackettools.validator.core; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; + +public class ValidationResultTest { + + @Test + public void serialize() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + mapper.enable(SerializationFeature.INDENT_OUTPUT); + + ValidatorInfo info = ValidatorInfo.of("ID", "NAME", "DESCRIPTION"); + ValidationResult result = ValidationResult.of(info, ValidationLevel.ERROR, "CATEGORY", "MESSAGE"); + + assertThat(mapper.writeValueAsString(result), + equalTo(""" + { + "validatorInfo" : { + "validatorId" : "ID", + "validatorName" : "NAME", + "description" : "DESCRIPTION" + }, + "level" : "ERROR", + "category" : "CATEGORY", + "message" : "MESSAGE" + }""".replaceAll("\n", System.lineSeparator())) + ); + } +} \ No newline at end of file diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidationResultsTest.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidationResultsTest.java new file mode 100644 index 000000000..939836385 --- /dev/null +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidationResultsTest.java @@ -0,0 +1,61 @@ +package org.phenopackets.phenopackettools.validator.core; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; + +public class ValidationResultsTest { + + @Test + public void serialize() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + mapper.enable(SerializationFeature.INDENT_OUTPUT); + + List validatorInfos = List.of( + ValidatorInfo.of("ID1", "NAME1", "DESCRIPTION1"), + ValidatorInfo.of("ID2", "NAME2", "DESCRIPTION2") + ); + List data = List.of( + ValidationResult.of(validatorInfos.get(0), ValidationLevel.ERROR, "CATEGORY1", "MESSAGE1"), + ValidationResult.of(validatorInfos.get(1), ValidationLevel.WARNING, "CATEGORY2", "MESSAGE2") + ); + ValidationResults results = ValidationResults.of(validatorInfos, data); + + assertThat(mapper.writeValueAsString(results), equalTo(""" + { + "validators" : [ { + "validatorId" : "ID1", + "validatorName" : "NAME1", + "description" : "DESCRIPTION1" + }, { + "validatorId" : "ID2", + "validatorName" : "NAME2", + "description" : "DESCRIPTION2" + } ], + "validationResults" : [ { + "validatorInfo" : { + "validatorId" : "ID1", + "validatorName" : "NAME1", + "description" : "DESCRIPTION1" + }, + "level" : "ERROR", + "category" : "CATEGORY1", + "message" : "MESSAGE1" + }, { + "validatorInfo" : { + "validatorId" : "ID2", + "validatorName" : "NAME2", + "description" : "DESCRIPTION2" + }, + "level" : "WARNING", + "category" : "CATEGORY2", + "message" : "MESSAGE2" + } ] + }""".replaceAll("\n", System.lineSeparator()))); + } +} \ No newline at end of file diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidatorInfoTest.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidatorInfoTest.java new file mode 100644 index 000000000..d1d2d4d69 --- /dev/null +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/ValidatorInfoTest.java @@ -0,0 +1,27 @@ +package org.phenopackets.phenopackettools.validator.core; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +public class ValidatorInfoTest { + + @Test + public void serialize() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + mapper.enable(SerializationFeature.INDENT_OUTPUT); + + ValidatorInfo info = ValidatorInfo.of("ID", "NAME", "DESCRIPTION"); + + assertThat(mapper.writeValueAsString(info), equalTo(""" + { + "validatorId" : "ID", + "validatorName" : "NAME", + "description" : "DESCRIPTION" + }""".replaceAll("\n", System.lineSeparator())) + ); + } +} \ No newline at end of file diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/AncestryHpoValidatorTest.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/AncestryHpoValidatorTest.java index aa5467d00..0671f0bbf 100644 --- a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/AncestryHpoValidatorTest.java +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/AncestryHpoValidatorTest.java @@ -3,7 +3,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.TestData; import org.phenopackets.phenopackettools.validator.core.ValidationLevel; @@ -19,7 +19,7 @@ public class AncestryHpoValidatorTest { - private static final Ontology HPO = TestData.HPO; + private static final MinimalOntology HPO = TestData.HPO; @Nested public class PhenopacketTest { @@ -49,7 +49,7 @@ public void testValidInput() { public void testFailsIfTermAndAncestorIsObserved() { // Has some Abnormality of finger and Arachnodactyly. Only Arachnodactyly should be present. Phenopacket pp = createPhenopacket( - "example-phenopacket", "example-subject", createPhenotypicFeature("HP:0001167", "Abnormality of finger", false), + "example-phenopacket", "example-subject", createPhenotypicFeature("HP:0001167", "Abnormal finger morphology", false), createPhenotypicFeature("HP:0001166", "Arachnodactyly", false) ).build(); @@ -60,14 +60,14 @@ public void testFailsIfTermAndAncestorIsObserved() { assertThat(result.validatorInfo(), equalTo(validator.validatorInfo())); assertThat(result.level(), equalTo(ValidationLevel.ERROR)); assertThat(result.category(), equalTo("Violation of the annotation propagation rule")); - assertThat(result.message(), equalTo("Phenotypic features of example-phenopacket must not contain both an observed term (Arachnodactyly, HP:0001166) and an observed ancestor (Abnormality of finger, HP:0001167)")); + assertThat(result.message(), equalTo("Phenotypic features of example-phenopacket must not contain both an observed term (Arachnodactyly, HP:0001166) and an observed ancestor (Abnormal finger morphology, HP:0001167)")); } @Test public void testFailsIfTermAndAncestorIsExcluded() { // Has neither Abnormality of finger nor Arachnodactyly. Only Abnormality of finger should be present. Phenopacket pp = createPhenopacket( - "example-phenopacket", "example-subject", createPhenotypicFeature("HP:0001167", "Abnormality of finger", true), + "example-phenopacket", "example-subject", createPhenotypicFeature("HP:0001167", "Abnormal finger morphology", true), createPhenotypicFeature("HP:0001166", "Arachnodactyly", true) ).build(); @@ -77,14 +77,14 @@ public void testFailsIfTermAndAncestorIsExcluded() { ValidationResult result = results.get(0); assertThat(result.level(), equalTo(ValidationLevel.ERROR)); assertThat(result.category(), equalTo("Violation of the annotation propagation rule")); - assertThat(result.message(), equalTo("Phenotypic features of example-phenopacket must not contain both an excluded term (Abnormality of finger, HP:0001167) and an excluded child (Arachnodactyly, HP:0001166)")); + assertThat(result.message(), equalTo("Phenotypic features of example-phenopacket must not contain both an excluded term (Abnormal finger morphology, HP:0001167) and an excluded child (Arachnodactyly, HP:0001166)")); } @Test public void testFailsIfTermIsPresentAndAncestorIsExcluded() { // Has neither Abnormality of finger nor Arachnodactyly. Only Abnormality of finger should be present. Phenopacket pp = createPhenopacket( - "example-phenopacket", "example-subject", createPhenotypicFeature("HP:0001167", "Abnormality of finger", true), + "example-phenopacket", "example-subject", createPhenotypicFeature("HP:0001167", "Abnormal finger morphology", true), createPhenotypicFeature("HP:0001166", "Arachnodactyly", false) ).build(); @@ -94,7 +94,7 @@ public void testFailsIfTermIsPresentAndAncestorIsExcluded() { ValidationResult result = results.get(0); assertThat(result.level(), equalTo(ValidationLevel.ERROR)); assertThat(result.category(), equalTo("Violation of the annotation propagation rule")); - assertThat(result.message(), equalTo("Phenotypic features of example-phenopacket must not contain both an observed term (Arachnodactyly, HP:0001166) and an excluded ancestor (Abnormality of finger, HP:0001167)")); + assertThat(result.message(), equalTo("Phenotypic features of example-phenopacket must not contain both an observed term (Arachnodactyly, HP:0001166) and an excluded ancestor (Abnormal finger morphology, HP:0001167)")); } } diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/HpoUniqueValidatorTest.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/HpoUniqueValidatorTest.java new file mode 100644 index 000000000..cad3ce4d7 --- /dev/null +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/HpoUniqueValidatorTest.java @@ -0,0 +1,171 @@ +package org.phenopackets.phenopackettools.validator.core.phenotype; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; +import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; +import org.phenopackets.phenopackettools.validator.core.TestData; +import org.phenopackets.phenopackettools.validator.core.ValidationLevel; +import org.phenopackets.phenopackettools.validator.core.ValidationResult; +import org.phenopackets.schema.v2.*; +import org.phenopackets.schema.v2.core.PhenotypicFeature; + +import java.util.List; + +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; +import static org.phenopackets.phenopackettools.validator.core.phenotype.Utils.createPhenopacket; +import static org.phenopackets.phenopackettools.validator.core.phenotype.Utils.createPhenotypicFeature; + +public class HpoUniqueValidatorTest { + + private static final MinimalOntology HPO = TestData.HPO; + + @Nested + public class PhenopacketTest { + + private PhenopacketValidator validator; + + @BeforeEach + public void setUp() { + validator = HpoPhenotypeValidators.Unique.phenopacketValidator(HPO); + } + + @Test + public void testValidInput() { + // Has some Abnormality of finger but no Arachnodactyly. + Phenopacket pp = createPhenopacket( + "example-phenopacket", "example-subject", + createPhenotypicFeature("HP:0001167", "Abnormality of finger", false), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", true) + ).build(); + + List results = validator.validate(pp); + + assertThat(results, is(empty())); + } + + @Test + public void testFailsIfObservedTermIsPresentTwice() { + // Has Arachnodactyly twice. + Phenopacket pp = createPhenopacket( + "example-phenopacket", "example-subject", + createPhenotypicFeature("HP:0001167", "Abnormality of finger", false), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", false), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", false) + ).build(); + + List results = validator.validate(pp); + + assertThat(results, hasSize(1)); + ValidationResult result = results.get(0); + assertThat(result.validatorInfo(), equalTo(validator.validatorInfo())); + assertThat(result.level(), equalTo(ValidationLevel.ERROR)); + assertThat(result.category(), equalTo("Non-unique phenotypic feature")); + assertThat(result.message(), equalTo(""" + Phenotypic features of example-phenopacket must not contain the same observed feature Arachnodactyly (HP:0001166) + more than once but the feature was present 2 times""")); + } + + @Test + public void testFailsIfExcludedTermIsPresentTwice() { + // Has excluded Arachnodactyly twice. + Phenopacket pp = createPhenopacket( + "example-phenopacket", "example-subject", + createPhenotypicFeature("HP:0001167", "Abnormality of finger", true), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", true), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", true) + ).build(); + + List results = validator.validate(pp); + + assertThat(results, hasSize(1)); + ValidationResult result = results.get(0); + assertThat(result.validatorInfo(), equalTo(validator.validatorInfo())); + assertThat(result.level(), equalTo(ValidationLevel.ERROR)); + assertThat(result.category(), equalTo("Non-unique phenotypic feature")); + assertThat(result.message(), equalTo(""" + Phenotypic features of example-phenopacket must not contain the same excluded feature Arachnodactyly (HP:0001166) + more than once but the feature was present 2 times""")); + } + } + + /** + * White-box testing - we know that the {@link PhenotypicFeature} is an attribute of a {@link Phenopacket}, so we + * test the validation logic extensively in {@link HpoUniqueValidatorTest.PhenopacketTest}. + * The {@link HpoUniqueValidatorTest.FamilyTest} test suite ensures there are not errors in valid input. + */ + @Nested + public class FamilyTest { + + private PhenopacketValidator validator; + + @BeforeEach + public void setUp() { + validator = HpoPhenotypeValidators.Unique.familyValidator(HPO); + } + + @Test + public void testValidInput() { + Family family = Family.newBuilder() + .setProband(createPhenopacket("example-phenopacket", "example-subject", + createPhenotypicFeature("HP:0001167", "Abnormality of finger", false), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", true)) + .build()) + .addRelatives(createPhenopacket("dad-phenopacket", "example-dad", + createPhenotypicFeature("HP:0001238", "Slender finger", false), + createPhenotypicFeature("HP:0100807", "Long fingers", false)) + .build()) + .addRelatives(createPhenopacket("mom-phenopacket", "example-mom", + createPhenotypicFeature("HP:0001238", "Slender finger", false), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", true)) + .build()) + .build(); + + List results = validator.validate(family); + + assertThat(results, is(empty())); + } + } + + /** + * White-box testing (same as in {@link HpoUniqueValidatorTest.FamilyTest}) - we know that the {@link PhenotypicFeature} + * is an attribute of a {@link Phenopacket}, so we test the validation logic extensively + * in {@link HpoUniqueValidatorTest.PhenopacketTest}. + * The {@link HpoUniqueValidatorTest.CohortTest} test suite ensures there are not errors in valid input. + */ + @Nested + public class CohortTest { + + private PhenopacketValidator validator; + + @BeforeEach + public void setUp() { + validator = HpoPhenotypeValidators.Ancestry.cohortHpoAncestryValidator(HPO); + } + + @Test + public void testValidInput() { + Cohort cohort = Cohort.newBuilder() + .addMembers(createPhenopacket("joe-phenopacket", "example-subject", + createPhenotypicFeature("HP:0001167", "Abnormality of finger", false), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", true)) + .build()) + .addMembers(createPhenopacket("jim-phenopacket", "example-jim", + createPhenotypicFeature("HP:0001238", "Slender finger", false), + createPhenotypicFeature("HP:0100807", "Long fingers", false)) + .build()) + .addMembers(createPhenopacket("jane-phenopacket", "example-jane", + createPhenotypicFeature("HP:0001238", "Slender finger", false), + createPhenotypicFeature("HP:0001166", "Arachnodactyly", true)) + .build()) + .build(); + + List results = validator.validate(cohort); + + assertThat(results, is(empty())); + } + } + +} \ No newline at end of file diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/OrganSystemValidatorTest.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/OrganSystemValidatorTest.java index 845fdc51d..d3a2e9b0f 100644 --- a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/OrganSystemValidatorTest.java +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/OrganSystemValidatorTest.java @@ -5,7 +5,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.monarchinitiative.phenol.ontology.data.TermId; import org.phenopackets.phenopackettools.validator.core.*; import org.phenopackets.schema.v2.*; @@ -20,7 +20,7 @@ public class OrganSystemValidatorTest { - private static final Ontology HPO = TestData.HPO; + private static final MinimalOntology HPO = TestData.HPO; private static final Set ABNORMALITY_OF_LIMBS_ORGAN_SYSTEM = Set.of(TermId.of("HP:0040064")); // Not a real organ system, but for the sake of testing... private static final Set SLENDER_FINGER_ORGAN_SYSTEM = Set.of(TermId.of("HP:0001238")); diff --git a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/PrimaryHpoPhenotypeValidatorTest.java b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/PrimaryHpoPhenotypeValidatorTest.java index 64e4ade80..41cfa9cbc 100644 --- a/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/PrimaryHpoPhenotypeValidatorTest.java +++ b/phenopacket-tools-validator-core/src/test/java/org/phenopackets/phenopackettools/validator/core/phenotype/PrimaryHpoPhenotypeValidatorTest.java @@ -4,7 +4,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.monarchinitiative.phenol.ontology.data.Ontology; +import org.monarchinitiative.phenol.ontology.data.MinimalOntology; import org.phenopackets.phenopackettools.validator.core.PhenopacketValidator; import org.phenopackets.phenopackettools.validator.core.TestData; import org.phenopackets.phenopackettools.validator.core.ValidationLevel; @@ -18,7 +18,7 @@ public class PrimaryHpoPhenotypeValidatorTest { - private static final Ontology HPO = TestData.HPO; + private static final MinimalOntology HPO = TestData.HPO; @Nested public class PhenopacketTest { @@ -67,8 +67,8 @@ public void testMissingTermId() throws Exception { }, "phenotypicFeatures": [{ "type": { - "id": "HP:0001182", - "label": "Tapered finger" + "id": "HP:0000000", + "label": "Nonexistent term" } }, { "type": { @@ -85,7 +85,7 @@ public void testMissingTermId() throws Exception { ValidationResult result = results.get(0); assertThat(result.level(), equalTo(ValidationLevel.ERROR)); assertThat(result.category(), equalTo("Invalid TermId")); - assertThat(result.message(), equalTo("HP:0001182 in proband A not found in 2021-06-08")); + assertThat(result.message(), equalTo("HP:0000000 in proband A not found in 2026-01-08")); } @Test diff --git a/phenopacket-tools-validator-core/src/test/resources/org/phenopackets/phenopackettools/validator/core/hp.v2026-01-08.json.gz b/phenopacket-tools-validator-core/src/test/resources/org/phenopackets/phenopackettools/validator/core/hp.v2026-01-08.json.gz new file mode 100644 index 000000000..fe11f489e Binary files /dev/null and b/phenopacket-tools-validator-core/src/test/resources/org/phenopackets/phenopackettools/validator/core/hp.v2026-01-08.json.gz differ diff --git a/phenopacket-tools-validator-jsonschema/pom.xml b/phenopacket-tools-validator-jsonschema/pom.xml index 89295ca12..73c58f079 100644 --- a/phenopacket-tools-validator-jsonschema/pom.xml +++ b/phenopacket-tools-validator-jsonschema/pom.xml @@ -7,7 +7,7 @@ org.phenopackets.phenopackettools phenopacket-tools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT phenopacket-tools-validator-jsonschema diff --git a/phenopacket-tools-validator-jsonschema/src/main/java/module-info.java b/phenopacket-tools-validator-jsonschema/src/main/java/module-info.java index 9d2de689e..09b55e53d 100644 --- a/phenopacket-tools-validator-jsonschema/src/main/java/module-info.java +++ b/phenopacket-tools-validator-jsonschema/src/main/java/module-info.java @@ -11,7 +11,8 @@ module org.phenopackets.phenopackettools.validator.jsonschema { requires org.phenopackets.phenopackettools.util; requires transitive org.phenopackets.phenopackettools.validator.core; - requires org.phenopackets.schema; + // Transitive due to export of Phenopacket, Cohort, Family in JsonSchemaValidationWorkflowRunner. + requires transitive org.phenopackets.schema; requires com.fasterxml.jackson.databind; requires json.schema.validator; requires org.slf4j; diff --git a/phenopacket-tools-validator-jsonschema/src/main/resources/org/phenopackets/phenopackettools/validator/jsonschema/v2/base.json b/phenopacket-tools-validator-jsonschema/src/main/resources/org/phenopackets/phenopackettools/validator/jsonschema/v2/base.json index f18df1331..21bc2041f 100644 --- a/phenopacket-tools-validator-jsonschema/src/main/resources/org/phenopackets/phenopackettools/validator/jsonschema/v2/base.json +++ b/phenopacket-tools-validator-jsonschema/src/main/resources/org/phenopackets/phenopackettools/validator/jsonschema/v2/base.json @@ -94,7 +94,8 @@ "properties": { "iso8601duration": { "description": "An ISO8601 string representing age.", - "type": "string" + "type": "string", + "pattern": "^P(?!$)(\\d+Y)?(\\d+M)?(\\d+W)?(\\d+D)?(T(?=\\d+[HMS])(\\d+H)?(\\d+M)?(\\d+S)?)?$" } }, "required": ["iso8601duration"], diff --git a/phenopacket-tools-validator-jsonschema/src/test/java/org/phenopackets/phenopackettools/validator/jsonschema/JsonSchemaValidationWorkflowRunnerTest.java b/phenopacket-tools-validator-jsonschema/src/test/java/org/phenopackets/phenopackettools/validator/jsonschema/JsonSchemaValidationWorkflowRunnerTest.java index b6e45e088..4d219f1c0 100644 --- a/phenopacket-tools-validator-jsonschema/src/test/java/org/phenopackets/phenopackettools/validator/jsonschema/JsonSchemaValidationWorkflowRunnerTest.java +++ b/phenopacket-tools-validator-jsonschema/src/test/java/org/phenopackets/phenopackettools/validator/jsonschema/JsonSchemaValidationWorkflowRunnerTest.java @@ -148,7 +148,6 @@ public void checkPhenotypicFeatureConstraints(String path, String action, String "/phenotypicFeatures[0]/onset/gestationalAge/weeks, SET[-1], 'phenotypicFeatures[0].onset.gestationalAge.weeks' must have a minimum value of 0", "/phenotypicFeatures[0]/onset/gestationalAge/days, SET[-1], 'phenotypicFeatures[0].onset.gestationalAge.days' must have a minimum value of 0", "/phenotypicFeatures[1]/onset/age/iso8601duration, DELETE, 'phenotypicFeatures[1].onset.age.iso8601duration' is missing but it is required", - // TODO - add test for ensuring that the duration is in an ISO8601 pattern "/phenotypicFeatures[2]/onset/ageRange/start, DELETE, 'phenotypicFeatures[2].onset.ageRange.start' is missing but it is required", "/phenotypicFeatures[2]/onset/ageRange/end, DELETE, 'phenotypicFeatures[2].onset.ageRange.end' is missing but it is required", // TODO - require end being at or after start @@ -160,6 +159,47 @@ public void checkTimeElementConstraints(String path, String action, String expec testErrors(runner, readBethlemPhenopacketNode(), path, action, expected, true); } + /** + * Check that error in the ISO8601 period leads to the appropriate + * {@link org.phenopackets.phenopackettools.validator.core.ValidationLevel#ERROR}. + */ + @ParameterizedTest + @CsvSource({ + // Age + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P1H], 'phenotypicFeatures[1].onset.age.iso8601duration' does not match the regex pattern ^P(?!$)(\\d+Y)?(\\d+M)?(\\d+W)?(\\d+D)?(T(?=\\d+[HMS])(\\d+H)?(\\d+M)?(\\d+S)?)?$", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[PT1D], 'phenotypicFeatures[1].onset.age.iso8601duration' does not match the regex pattern ^P(?!$)(\\d+Y)?(\\d+M)?(\\d+W)?(\\d+D)?(T(?=\\d+[HMS])(\\d+H)?(\\d+M)?(\\d+S)?)?$", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P], 'phenotypicFeatures[1].onset.age.iso8601duration' does not match the regex pattern ^P(?!$)(\\d+Y)?(\\d+M)?(\\d+W)?(\\d+D)?(T(?=\\d+[HMS])(\\d+H)?(\\d+M)?(\\d+S)?)?$", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[PT], 'phenotypicFeatures[1].onset.age.iso8601duration' does not match the regex pattern ^P(?!$)(\\d+Y)?(\\d+M)?(\\d+W)?(\\d+D)?(T(?=\\d+[HMS])(\\d+H)?(\\d+M)?(\\d+S)?)?$", + // Wrong token order. + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P10M4Y], 'phenotypicFeatures[1].onset.age.iso8601duration' does not match the regex pattern ^P(?!$)(\\d+Y)?(\\d+M)?(\\d+W)?(\\d+D)?(T(?=\\d+[HMS])(\\d+H)?(\\d+M)?(\\d+S)?)?$", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P1YT4M12H], 'phenotypicFeatures[1].onset.age.iso8601duration' does not match the regex pattern ^P(?!$)(\\d+Y)?(\\d+M)?(\\d+W)?(\\d+D)?(T(?=\\d+[HMS])(\\d+H)?(\\d+M)?(\\d+S)?)?$", + }) + public void checkTimeElementConstraints_ISO8601_errors(String path, String action, String expected) { + testErrors(runner, readBethlemPhenopacketNode(), path, action, expected, true); + } + + /** + * Check that correct ISO8601 period yields no + * {@link org.phenopackets.phenopackettools.validator.core.ValidationLevel#ERROR}s. + */ + @ParameterizedTest + @CsvSource({ + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P1Y4M3W2DT1H23M44S]", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P1Y]", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P4M]", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P3W]", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[P2D]", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[PT1H]", + "/phenotypicFeatures[1]/onset/age/iso8601duration, SET[PT1H2M]", + }) + public void checkTimeElementConstraints_ISO8601_OK(String path, String action) { + JsonNode tampered = TAMPERER.tamper(readBethlemPhenopacketNode(), path, Action.valueOf(action)); + + ValidationResults results = runner.validate(tampered.toPrettyString()); + + assertThat(results.isValid(), equalTo(true)); + } + /** * Absence of phenotypic feature id leads to an {@link org.phenopackets.phenopackettools.validator.core.ValidationLevel#ERROR}. */ @@ -683,10 +723,6 @@ private static void testErrors(ValidationWorkflowRu JsonNode tampered = TAMPERER.tamper(node, path, Action.valueOf(action)); ValidationResults results = runner.validate(tampered.toPrettyString()); - // TODO - remove after the tests are completed -// results.validationResults().stream() -// .map(ValidationResult::message) -// .forEach(System.err::println); Collection tokens = Arrays.asList(errors.split("\\|")); if (validateCount) { diff --git a/pom.xml b/pom.xml index dd2a2a31a..b18d34419 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.phenopackets.phenopackettools phenopacket-tools - 1.0.0-RC3 + 1.0.0-RC4-SNAPSHOT Phenopacket-tools An app and library for building, conversion and validation of GA4GH Phenopackets @@ -15,7 +15,7 @@ https://phenopackets.org/phenopacket-tools - 3.6.0 + 3.6.3 @@ -92,7 +92,7 @@ 17 3.21.7 2.0.2 - 2.0.0 + 2.1.1 2.14.2 1.0.79 2.0 @@ -116,7 +116,7 @@ org.apache.maven.plugins maven-surefire-plugin - 3.0.0-M5 + 3.5.0 false