buckaroo icon indicating copy to clipboard operation
buckaroo copied to clipboard

Better message for cycles in analysis graph

Open paddymul opened this issue 9 months ago • 2 comments

Checks

  • [x] I have checked that this issue has not already been reported.

  • [x] I have confirmed this bug exists on the latest version of Buckaroo.

What type of jupyter notebook were you using (VSCode notebook, google colab, Jupyter Lab, Jupyter notebook). Select multiple if you can reproduce this in multiple environments. If other, please add to description.

Jupyter Lab

Reproducible example

class CycleA(ColAnalysis):
    provides_defaults = {'cycle_a': 'asdf'}
    requires_summary = ["cycle_b"]

class CycleB(ColAnalysis):
    provides_defaults = {'cycle_b': 'foo'}
    requires_summary = ["cycle_a"]

Issue description

the above code displays an error message of

e ('nodes are in a cycle', ['CycleA###cycle_a', 'cycle_a', 'CycleB###cycle_b', 'cycle_b', 'CycleA###cycle_a'])
graph {'CycleA###cycle_a': {'cycle_b'}, 'cycle_a': {'CycleA###cycle_a'}, 'CycleB###cycle_b': {'cycle_a'}, 'cycle_b': {'CycleB###cycle_b'}}
e ('nodes are in a cycle', ['CycleA###cycle_a', 'cycle_a', 'CycleB###cycle_b', 'cycle_b', 'CycleA###cycle_a'])

Expected behavior

Those are very simple classes and it is still hard to figure out what's going on. A better error message is needed

Installed versions

``` dev version
</details>


### Jupyter Log output

```shell

paddymul avatar Mar 10 '25 15:03 paddymul

Some explanatory code for understanding the graph structure


class CycleA(ColAnalysis):
    provides_defaults = {'cycle_a': 'asdf'}
    requires_summary = ["cycle_b"]

class CycleB(ColAnalysis):
    provides_defaults = {'cycle_b': 'foo'}
    requires_summary = ["cycle_a"]
{'CycleA###cycle_a': {'cycle_b'},
 'cycle_a': {'CycleA###cycle_a'},
 'CycleB###cycle_b': {'cycle_a'},
 'cycle_b': {'CycleB###cycle_b'}}

class CycleA depends on prop cycle_b
prop cycle_a depends on CycleA
class CyclcleB depends on prop cycle_a
prop cycle_b depends on class CycleA
class CycleA(ColAnalysis):
    provides_defaults = {'cycle_a': 'asdf', 'extra_from_class_a':'asdf'}
    requires_summary = ["cycle_b"]

class CycleB(ColAnalysis):
    provides_defaults = {'cycle_b': 'foo'}
    requires_summary = ["cycle_a"]

{'CycleA###cycle_a': {'CycleA###extra_from_class_a'},
 'CycleA###extra_from_class_a': {'cycle_b'},
 'cycle_a': {'CycleA###cycle_a'},
 'extra_from_class_a': {'CycleA###cycle_a'},
 'CycleB###cycle_b': {'cycle_a'},
 'cycle_b': {'CycleB###cycle_b'}}
class NeedsB(ColAnalysis):
    provides_defaults = {'a': 'asdf'}
    requires_summary = ["b"]

class ProvidesB(ColAnalysis):
    provides_defaults = {'b': 'foo'}

{'ProvidesB###b': set(),
 'b': {'ProvidesB###b'},
 'NeedsB###a': {'b'},
 'a': {'NeedsB###a'}}

paddymul avatar Mar 10 '25 17:03 paddymul

For now I have added detection of SelfCycle where an analysis requires a key that it provides. That is a particularly confusing case.

Ideally I'd like an error that says "CycleA dependson CycleB for 'b'" "CycleB depends on CycleA for 'a'" And I would like it to ignore extra_from_class_a because that isn't part of the cycle

The raw graph is hard for users to understand.

paddymul avatar Mar 10 '25 17:03 paddymul