NCATSTranslator / Feedback

A repo for tracking gaps in Translator data and finding ways to fill them.
7 stars 0 forks source link

TRAPI Warnings: attribute_type_id not an association slot #815

Open cbizon opened 5 months ago

cbizon commented 5 months ago

See #811

=> Edge has an attribute_type_id that is not an association slot
            $ infores:bindingdb -> infores:biothings-bindingdb -> infores:service-provider-trapi
                # biolink:provided_by
                # biolink:iri

It looks like service provider is returning attribute_type_ids that are not association slots.

@sierra-moxon should these be slots? Is this an ignorable warning or should service provider choose something else?

newgene commented 5 months ago

looping in @colleenXu for Service Provider

colleenXu commented 4 months ago

I don't think biolink:provided_by or biolink:iri is coming from Service Provider/BioThings BindingDB. Instead, I think it's coming from Spoke alone.

It's the same reasoning as https://github.com/NCATSTranslator/Feedback/issues/814#issuecomment-2216896382, and those example edges also have these edge-attributes.

sierra-moxon commented 4 months ago

@brettasmi - biolink:provided_by is a node property in Biolink.
If you want to talk about who provided the edge, use "biolink:knowledge_source"

node vs. edge provenance was discussed in DM calls ~ 2022, here's a ticket that summarizes some of those discussions: https://github.com/biolink/biolink-model/issues/938

We kept biolink:provided_by as a node property in order to catch cases where folks wanted to be able to track "node provenance" as well.

sierra-moxon commented 4 months ago

In terms of biolink:iri it isn't currently specified as a node or edge property in biolink. It is unclear if this needs to be assigned as one or the other. I think in cases where a property is not a child of either "edge property" or "node property", the validator shouldn't generate an error.

sstemann commented 3 weeks ago

@sierra-moxon what is the expected change here? can you verify?

RichardBruskiewich commented 2 weeks ago

In terms of biolink:iri it isn't currently specified as a node or edge property in biolink. It is unclear if this needs to be assigned as one or the other. I think in cases where a property is not a child of either "edge property" or "node property", the validator shouldn't generate an error.

@sstemann I think @sierra-moxon is suggesting a tweak to validation that lets a few fringe cases through. I'll see what I can do to patch this over the next few days.

RichardBruskiewich commented 2 weeks ago

Just a quick observation: the reasoner-validator doesn't actually care about validating attributes on nodes, but only edges., even though it would appear in TRAPI 1.5 that node attributes are mandatory (but can be an empty array.

        attributes:
          type: array
          description: A list of attributes describing the node
          items:
            $ref: '#/components/schemas/Attribute'
          minItems: 0
          nullable: false

If present, one wonders if the attribute_type_id of the attributes should be node_property slot valiues (or not?)

RichardBruskiewich commented 2 weeks ago

Rereading your comments about biolink:iri above, @sierra-moxon , I guess the one tweak here is simply to add the additional condition whether an (edge) attribute_type_id is a node_property, then complaining... otherwise, not. Fine.. I can do that. The code in question:

                                            elif not self.bmt.is_association_slot(attribute_type_id):
                                                self.report(
                                                    code="warning.knowledge_graph.edge." +
                                                         "attribute.type_id.not_association_slot",
                                                    identifier=attribute_type_id,
                                                    edge_id=edge_id,
                                                    source_trail=source_trail
                                                )

is only run in the context of edges, so iI would replace with:

                                            elif self.bmt.is_node_property(attribute_type_id):
                                                self.report(
                                                    code="warning.knowledge_graph.edge." +
                                                         "attribute.type_id.not_association_slot",
                                                    identifier=attribute_type_id,
                                                    edge_id=edge_id,
                                                    source_trail=source_trail
                                                )