codecov[bot] commented 3 years ago

Codecov Report

Merging #46 (08fa85d) into main (ca0fe3c) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #46   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          401       401           
=========================================
  Hits           401       401

Impacted Files	Coverage Δ
reasoner_transpiler/matching.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ca0fe3c...08fa85d. Read the comment docs.

YaphetKG commented 3 years ago

tested out with, Plater looks good.

YaphetKG commented 3 years ago

I was testing this with plater which has a dependency of bmt installed. I think there might be a difference in how Toolkit.get_decendants behaves in bmt vs bmt-lite (?) ,

When reasoner transplier get to https://github.com/ranking-agent/reasoner-transpiler/blob/ca0fe3c81433fb3323416ec339ddce431ed53874/reasoner_transpiler/matching.py#L177 , plater's installation seems to filter out every edge , resulting an empty list that would cause match all edges. But i don't think that's the case when using bmt-lite for as seen in the tests here.

I was wondering if there is difference in implementation of bmt-lite vs bmt (Toolkit.get_descendants) causing this

patrickkwang commented 3 years ago

Yes, bmt and bmt-lite are slightly different. One difference in the latest version involves how they handle reflexive descendants of unknown predicates. reasoner-transpiler counts on receiving the unknown predicate back, which bmt-lite does, while bmt returns an empty list for an unknown predicate regardless of whether reflexive=True. A better behavior than either may be to raise an error: https://github.com/biolink/biolink-model-toolkit/issues/48

patrickkwang commented 3 years ago

Also, bmt and bmt-lite cannot co-exist since they both provide the package "bmt".

YaphetKG commented 3 years ago

oh ok, we were depending on bmt for attribute resolution in the Plater code, where attribute names of nodes and edges are looked up on biolink using bmt and their slot_uri would be used if a match was found. I tried to use bmt-lite lib for this but it was causing errors as the whole information about biolink classes is not returned. this https://github.com/TranslatorSRI/Plater/blob/17be4648dc343e1669990246d424ad38bcf78e52/PLATER/services/util/question.py#L78 would raise errors when using bmt-lite, I think there is a difference between what is returned .

Is there another way to help this filtering ?

patrickkwang commented 3 years ago

We can totally work on filling out bmt-lite based on whatever you need. What was the error you encountered?

YaphetKG commented 3 years ago

  File "C:\Users\19193\work\Translator-SRI\Plater\venv\lib\site-packages\fastapi\routing.py", line 216, in app
    raw_response = await run_endpoint_function(
  File "C:\Users\19193\work\Translator-SRI\Plater\venv\lib\site-packages\fastapi\routing.py", line 149, in run_endpoint_function
    return await dependant.call(**values)
  File "C:\Users\19193\work\Translator-SRI\Plater\PLATER\services\app_trapi_1_1.py", line 32, in reasoner_api
    response = await question.answer(graph_interface)
  File "C:\Users\19193\work\Translator-SRI\Plater\PLATER\services\util\question.py", line 127, in answer
    self._question_json.update(self.transform_attributes(results_dict[0], graph_interface))
  File "C:\Users\19193\work\Translator-SRI\Plater\PLATER\services\util\question.py", line 110, in transform_attributes
    self.format_attribute_trapi_1_1(trapi_message.get('knowledge_graph', {}).get('nodes', {}), graph_interface)
  File "C:\Users\19193\work\Translator-SRI\Plater\PLATER\services\util\question.py", line 78, in format_attribute_trapi_1_1
    if 'slot_uri' in bl_info:
TypeError: argument of type 'ClassDefinition' is not iterable

After calling

 bl_info = graph_interface.toolkit.get_element(attr['original_attribute_name'])

bl_info would contain id_prefixes and name Plater's code looks for attrribute type id as bl_info['slot_uri'] then use bl_info['range'] , if range is available looks for new_type =graph_interface.toolkit.get_element(bl_info['range']) and use new_type['uri'] as value type id and lastly it defaults to bl_info['class_uri'] if slot_uri is not available

YaphetKG commented 3 years ago

Fix confirmed. works great thanks

ranking-agent / reasoner-transpiler

Biolink 2.1.0 #46

Codecov Report