Skip to content

Commit 7f75010

Browse files
committed
Force Graph to actually use frozensets
Because type annotations in python are a "I wish people would respect my authority" kind of feature, we cannot guarantee that a `Graph` is actually created with frozensets as attributes. That is the case all throughout taskgraph where we're passing sets. By forcing those attributes to be frozensets we can ensure that `Graph` is hashable which will allow us to use `functools.lru_cache` to cache some expensive functions in here. Unfortunately, there's no easy way of transforming the inputs passed into a dataclass, since in both `__init__` and `__post_init__`, the structure is already frozen. By using a dataclass as the parent of `Graph` instead of `Graph` itself, we can work around the issue while keeping the frozenness of the dataclass post init without having to resort to working around dataclass when transforming the attributes
1 parent 8c8488d commit 7f75010

File tree

4 files changed

+14
-7
lines changed

4 files changed

+14
-7
lines changed

src/taskgraph/graph.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@
88

99

1010
@dataclass(frozen=True)
11-
class Graph:
11+
class _Graph:
12+
nodes: frozenset
13+
edges: frozenset
14+
15+
16+
class Graph(_Graph):
1217
"""Generic representation of a directed acyclic graph with labeled edges
1318
connecting the nodes. Graph operations are implemented in a functional
1419
manner, so the data structure is immutable.
@@ -23,8 +28,8 @@ class Graph:
2328
node `left` to node `right`..
2429
"""
2530

26-
nodes: frozenset
27-
edges: frozenset
31+
def __init__(self, nodes, edges):
32+
super().__init__(frozenset(nodes), frozenset(edges))
2833

2934
def transitive_closure(self, nodes, reverse=False):
3035
"""Return the transitive closure of <nodes>: the graph containing all
@@ -67,7 +72,7 @@ def transitive_closure(self, nodes, reverse=False):
6772
add_nodes = {(left if reverse else right) for (left, right, _) in add_edges}
6873
new_nodes = nodes | add_nodes
6974
new_edges = edges | add_edges
70-
return Graph(new_nodes, new_edges) # type: ignore
75+
return Graph(new_nodes, new_edges)
7176

7277
def _visit(self, reverse):
7378
queue = collections.deque(sorted(self.nodes))

src/taskgraph/morph.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def amend_taskgraph(taskgraph, label_to_taskid, to_add):
5151
for depname, dep in task.dependencies.items():
5252
new_edges.add((task.task_id, dep, depname))
5353

54-
taskgraph = TaskGraph(new_tasks, Graph(set(new_tasks), new_edges)) # type: ignore
54+
taskgraph = TaskGraph(new_tasks, Graph(frozenset(new_tasks), new_edges))
5555
return taskgraph, label_to_taskid
5656

5757

src/taskgraph/optimize/base.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,9 @@ def get_subgraph(
451451
if left in tasks_by_taskid and right in tasks_by_taskid
452452
}
453453

454-
return TaskGraph(tasks_by_taskid, Graph(set(tasks_by_taskid), edges_by_taskid)) # type: ignore
454+
return TaskGraph(
455+
tasks_by_taskid, Graph(frozenset(tasks_by_taskid), edges_by_taskid)
456+
)
455457

456458

457459
@register_strategy("never")

src/taskgraph/taskgraph.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,5 +67,5 @@ def from_json(cls, tasks_dict):
6767
tasks[key].task_id = value["task_id"]
6868
for depname, dep in value["dependencies"].items():
6969
edges.add((key, dep, depname))
70-
task_graph = cls(tasks, Graph(set(tasks), edges)) # type: ignore
70+
task_graph = cls(tasks, Graph(frozenset(tasks), edges))
7171
return tasks, task_graph

0 commit comments

Comments
 (0)