njwilson23 / depgraph

Dependency resolution for datasets
MIT License
5 stars 4 forks source link

buildall() does not work for simplest graph #3

Open noblige opened 3 years ago

noblige commented 3 years ago

https://github.com/njwilson23/depgraph/blob/e73dfdfe4ac618f46b331c05f95307fed1bfe07b/depgraph/depgraph.py#L331 should this have roots added to the list? E.g.:

            ...
            i, dep = queue.pop(0)
            marks[dep] = i
            ...

Otherwise I'm not seeing any code that would produce items that don't have any parents? Simple code:

from depgraph import Dataset, buildall
parent = Dataset('parent-does-not-exists', tool=None)
child = Dataset('child-does-not-exists', tool=None)
child.dependson(parent)
for stage in buildall(child):
    for dep, reason in stage:
        print("Building {0} with {1} because {2}".format(dep.name, dep.tool, reason))

Output only builds child, not the parent:

Building child-does-not-exists with None because the dataset doesn't exist
njwilson23 commented 3 years ago

Perhaps! I think every case where I've used this, the inputs (e.g. a dataset, source code, a config file) have already existed. Being able to generate a parent from nothing is an interesting idea, and I agree the current behaviour isn't right.

And thanks for the patch! I'll take a closer look and get back to you in a bit.