schemaview.py: add cycle detection function #469
Conversation
…the _closure function
ialarmedalien
requested review from
Silvanoc,
dalito,
kevinschaper and
sierra-moxon
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #469 +/- ##
==========================================
+ Coverage 77.59% 77.65% +0.06%
==========================================
Files 52 52
Lines 4459 4480 +21
Branches 968 973 +5
==========================================
+ Hits 3460 3479 +19
- Misses 778 779 +1
- Partials 221 222 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| Uses the classic white/grey/black colour coding algorithm to track which nodes have been explored. In this | ||
| case, "node" refers to any element in a schema and "neighbours" are elements that can be reached from that | ||
| node by executing function `f`. | ||
| WHITE: unexplored | ||
| GREY: node is being processed; processing includes exploring all neighbours reachable via f(node) | ||
| BLACK: node and all of its neighbours (and their neighbours, etc.) have been processed |
There was a problem hiding this comment.
This naming system could be changed (e.g. to TODO / PROCESSING / DONE or something similar) but the white/grey/black nomenclature is used in numerous places where this algorithm is described, so may as well stick with it.
| if kwargs and kwargs.get("detect_cycles"): | ||
| detect_cycles(f, x) |
There was a problem hiding this comment.
allows users run schemaview.type_descendants("my_type", detect_cycles=True) or schemaview.class_ancestors("CoolClass", detect_cycles=True)
|
|
||
|
|
||
| @pytest.mark.parametrize(("target", "cycle_start_node"), [(k, v) for k, v in CYCLES[TYPES][0].items()]) | ||
| @pytest.mark.parametrize("fn", ["detect_cycles", "graph_closure", "type_ancestors"]) |
There was a problem hiding this comment.
test calling the function directly, via the graph closure (_closure function), and through SchemaView methods that use _closure
| def check_recursive_id_slots(class_name: str) -> list[str]: | ||
| """Given a class, retrieve any classes used as the range for the class identifier slot.""" | ||
| id_slot = sv_cycles_schema.get_identifier_slot(class_name, use_key=True) | ||
| if not id_slot: | ||
| return [] | ||
| ind_range = sv_cycles_schema.slot_range_as_union(id_slot) | ||
| return [sv_cycles_schema.get_class(x).name for x in ind_range if sv_cycles_schema.get_class(x)] or [] |
There was a problem hiding this comment.
use detect_cycles with a custom function -- here I'm checking whether I will enter recursion hell whilst trying to ascertain the type(s) allowed by a slot with a class as the range.
2 participants
This PR adds a function to detect cycles when recursively executing a function, e.g. when retrieving class ancestors and repeatedly executing
schemaview.class_parents(...).The
_closurefunction allows one to recursively execute functions without getting caught in cycles, but it is also useful to see if cycles do exist as they may represent errors in the schema logic that need to be fixed. Adding the kwargsdetect_cyclesto the_closurefunction or any functions that call it under the hood (e.g.class_ancestors) will execute the cycle detection algorithm before generating the closure.It would be possible to integrate this function into the
_closurefunction -- the main reason for not doing so is that it would make the_closurefunction more complicated and less readable. The performance benefits are unlikely to be worth it unless someone is dealing with a massive schema with a very complex inheritance hierarchy (and even then...).It may be worth moving both the
_closureanddetect_cyclesfunctions into a separate module at some point if it seems as though there might be more call for graph-type algorithms.