elixir-europe / biovalidator

JSON validator derived from AJV supporting ontology and taxonomy validation.
Apache License 2.0
20 stars 6 forks source link

[BUG]: NCBITaxon wrong ontology assertion: ``...not child of...`` #58

Closed M-casado closed 1 year ago

M-casado commented 1 year ago

Bug summary

When using the custom keyword graphRestriction for the NCBITaxon ontology, it seems to fail regardless of the set parent level.

Technical details

To reproduce

  1. Clone, install and deploy a local server
    git clone https://github.com/elixir-europe/biovalidator.git
    cd biovalidator
    npm install
    node src/biovalidator
  2. Regardless of endpoint (UI or CLI), set the schema to use graphRestriction with NCBITaxon ontology; and the data to correctly contain the CURIE of a term that is hierarchically below the one in the custom keyword.
    
    # In this case the schema constraint is any NCBITaxon CURIE below its root level (NCBITaxon:1).
    {
    "type": "object",
    "required" : ["taxonIdCurie"],
    "properties": {
    "taxonIdCurie": {
      "type": "string",
      "graphRestriction":  {
        "ontologies" : ["obo:NCBITAXON"],
        "classes": ["NCBITaxon:1"],
        "relations": ["rdfs:subClassOf"],
        "direct": false,
        "include_self": false
      }
    }
    }
    }

And the data contains the NCBITaxon CURIE for humans

{ "taxonIdCurie": "NCBITaxon:9606" }

3. Observe the erroneous message:

Observed behaviour

The validation does not pass even though the conditions are met for it to pass (i.e. the term is correct for the hierarchy)

Expected behaviour

The validation result should be that it was passed.

Additional context

I'm unsure if this error has to do with the tool itself, OLS API, or NCBITaxon per se. But it is worth investigating.

I also checked with other levels of the hierarchy:

M-casado commented 1 year ago

Similar to the other issue (see https://github.com/elixir-europe/biovalidator/issues/56#issuecomment-1398710137), we ended up adapting our schemas to avoid this one, but this time it is indeed a wacky way to fix it and in the future we would very much prefer for the validation to take place for this term.

theisuru commented 1 year ago

@M-casado NCBITaxon_9606 in the OLS the ontology tree is not shown correctly. I am not sure if this is a recent problem or this is intentional. I will ask OLS team and get back to you.

theisuru commented 1 year ago

Did some more digging before contacting OLS team. It looks like ontologies in the graphRestriction are case sensitive. Same query works if we used obo:ncbitaxon instead of obo:NCBITAXON.

If all the ontologies are in simple case in OLS, I can make the transformation in the code aswell or leave it as it is for the user to figure out the correct case. What do you think?

M-casado commented 1 year ago

Ah I see, nice catch, @theisuru. Regarding your question, it would depend on whether NCBITaxon is another ontology that can be used. If OLS always uses lowercase and the uppercase is thrown under the bin, then perhaps the lower case is justified.

Nevertheless, a simple mention in the documentation would also suffice for me. I'll address this myself as well in a short PR.