Fix: nodes at t

[BadPrograms]

If some of the roots existed in t while others started later, the ones that existed would not be added to the result.

[leoguignard]

Different proposition on how to solve the bug, and some small suggestions on how to improve the code

[leoguignard]

My suggestion, keeping as is, only changing the while:

while 0 < len(to_do):
    curr = to_do.pop()
    if lT._time[curr] == t:
        final_nodes.append(curr)
    elif lT._time[curr] < t:
        to_do.extend(lT.successor[curr])

I think it also allows the removal of

if not final_nodes:
    return list(r)

[leoguignard]

Also I guess one could do it that way at the beginning:

if r is None:
    ...

Instead of

if not r and r != 0:
    ...

[leoguignard]

And yes, it would change the behavior since it would make the function return nothing in case the user gave [] for roots instead of returning the output of lT.roots which is I think a plus.
If the user specifically gave the empty list of nodes, it should not get any node in the return value.

[BadPrograms]

Since we have time_nodes again, I don't care about the behaviour, so I will do that.

[BadPrograms]

Actually since we have the time_nodes the function should change entirely.

[BadPrograms]

Also, I think we might want to return a set and not a list. There is no reason to index the result, and I like sets more. The only reason to use a list is that the rest of the LineageTree consists mostly of lists, so we can do so for the sake of consistency.

[leoguignard]

Also I guess one could do it that way at the beginning:

if r is None:
...

Instead of

if not r and r != 0:
...

Dont you want to do that?

[leoguignard]

Why should the function change? and what should it be instead?

About the list vs set, the list can have an advantadge since it is explicitly ordered and that currently the nodes are returned in a specific order (preorder)

[BadPrograms]

If we dont provide roots it should return lT.time_nodes[t]

[leoguignard]

Also I guess one could do it that way at the beginning:

if r is None:
...

Instead of

if not r and r != 0:
...

Dont you want to do that?

[leoguignard]

Why should the function change? and what should it be instead?

About the list vs set, the list can have an advantadge since it is explicitly ordered and that currently the nodes are returned in a specific order (preorder)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.91%. Comparing base (93ee2ca) to head (994a0f9).
⚠️ Report is 12 commits behind head on code-split-3.

Additional details and impacted files

@@              Coverage Diff              @@
##           code-split-3      #85   +/-   ##
=============================================
  Coverage         96.91%   96.91%           
=============================================
  Files                 2        2           
  Lines               356      356           
  Branches             17       17           
=============================================
  Hits                345      345           
  Misses                6        6           
  Partials              5        5           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[leoguignard]

added to code-split-4

Closing for that reason