biolink / ontobio

python library for working with ontologies and ontology associations
https://ontobio.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
119 stars 30 forks source link

automatically fix obsolete GO terms in the with/from field #593

Closed sierra-moxon closed 2 years ago

sierra-moxon commented 2 years ago

Thank you so much for taking the time to look through this! 😁 - happy to refactor.
first half of fixing: https://github.com/geneontology/go-site/issues/2110

I had to fix up a POMBASE GAF parsing test - since the with/from gets passed through the _validate_ontology_class_id method, it warns/reports when a GO id isn't found in the ontology associated with the parser. I'm not sure why the reported ids wouldn't be in the go-truncated-pombase.json?

When I run 'make test' locally, I see failing tests that don't show up in the github actions/PR testing. Should I worry about local test failures like this one?

test_ecomap.py::test_ecomap FAILED                                       [100%]
tests/test_ecomap.py:3 (test_ecomap)
ECO:0007669 != ECO:0000501

Expected :ECO:0000501
Actual   :ECO:0007669
<Click to see difference>

def test_ecomap():
        """
        test mappings between GAF codes and ECO
        """
        m = EcoMap()
        assert m.coderef_to_ecoclass('IEA', 'GO_REF:0000002') == 'ECO:0000256'
>       assert m.coderef_to_ecoclass('IEA') == 'ECO:0000501'
E       AssertionError: assert 'ECO:0007669' == 'ECO:0000501'
kltm commented 2 years ago

@sierra-moxon I'm wondering if it's chance that this is an ECO/IEA error that you're getting, considering recent changes. I think we need to track down the test issue: either removing it or fixing it. Otherwise, everything seems good to me. @dustine32 may have more insightful thoughts.

dustine32 commented 2 years ago

Correct @kltm, that test failure is due to IEA for reals not being ECO:0000501 anymore (it's now ECO:0007669). Not sure what to do with this test on the EcoMap functionality, since @cmungall advises against hard-coding ECO codes. Maybe either remove these tests or make a dummy mapping (that won't change) to pass into ecomap._mappings?

@sierra-moxon Usually, I'll just run make travis_test, which is a smaller set of tests and what travis runs in GH. I believe this test_ecomap.py is skipped in make travis_test.