jhu-bids / TermHub

Web app and CLI tools for working with biomedical terminologies. https://github.com/orgs/jhu-bids/projects/9/views/7
https://bit.ly/termhub
GNU General Public License v3.0
8 stars 10 forks source link

`test_get_missing_in_between_nodes()` upgrade #784

Open joeflack4 opened 1 month ago

joeflack4 commented 1 month ago

Overview

[!warning] Don't start this until #749 is merged, as there is a necessary refactor in there.

Write tests.

Siggie said: feel free to grab them if needed to explain.

Original OP concerning bugs found

Siggie wrote: > I discovered a bug in get_missing_in_between_nodes > We need to figure out why our existing tests didn't catch it and fix them. > ```python > from backend.routes.graph import get_missing_in_between_nodes, REL_GRAPH > x = get_missing_in_between_nodes(REL_GRAPH, [4146581, 317009, 257581], verbose=True) > > x = get_missing_in_between_nodes(REL_GRAPH, [4146581, 317009], verbose=True) > ``` > Two bugs, actually. > 317009 is an ancestor of 4146581, but this call returns nothing in between. The problem there is (I think) that 317009 is considered a leaf because nothing is connected to it in this 2-item subgraph. Adding 257581 makes it so 317009 is seen as a root. > But it still doesn't work. I haven't figured out why yet. > Crap... this algorithm is tricky. I've spent a couple hours this morning on it and am kind of baffled. It needs better documentation and I probably need help debugging it.

Sub-tasks

Can probably use TDD to get both done at same time.

joeflack4 commented 1 month ago

I was thinking this would be a good opportunity for TDD, so I made a single issue for both, but Siggie completed the bug fixes already and I don't think they saw my message, so since all that remains is the test, I renamed the issue.