Skip to content

Remove relation edge attribute and reporter edges#63

Merged
dgkf-roche merged 3 commits intodevfrom
dev-rm-edge-relation-attr
Jun 20, 2025
Merged

Remove relation edge attribute and reporter edges#63
dgkf-roche merged 3 commits intodevfrom
dev-rm-edge-relation-attr

Conversation

@dgkf-roche
Copy link
Collaborator

Will highlight specific considerations in comments

Comment on lines -223 to 237
start_task = function(task) {
task_graph_package_status(self$graph, task) <- STATUS$`in progress`
start_node = function(node) {
task_graph_package_status(self$graph, node) <- STATUS$`in progress`
private$start_node_meta_parents(node)
},

start_node_meta_parents = function(node) {
# start parent meta tasks recursively once a child starts
parent_nodes <- igraph::adjacent_vertices(self$graph, node, mode = "in")
parent_nodes <- unlist(parent_nodes, use.names = FALSE)
parent_nodes <- V(self$graph)[parent_nodes]
meta_parent_nodes <- parent_nodes[is_meta(parent_nodes$task)]
for (meta in meta_parent_nodes) {
task_graph_package_status(self$graph, meta) <- STATUS$`in progress`
private$start_node_meta_parents(meta)
}
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the most important change. When new processes are started or finished, the status of parent meta tasks are also updated (recursively).

If a meta task has any children that are active, it will be active. Once all children are finished, it also finishes.

This made it pretty straightforward to get everything working with the reporter without the relation edges attributes.

Comment on lines -231 to +260
push_process = function(task, x) {
task_graph_task_process(self$graph, task) <- x
name <- task_graph_task_name(self$graph, task)
private$start_task(task)
push_process = function(node, x) {
task_graph_task_process(self$graph, node) <- x
name <- task_graph_task_name(self$graph, node)
private$start_node(node)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More of a refactor than a substantive change, but I found this hard to reason about because task is actually a vertex. I felt like this would make it a bit easier to develop against.

@dgkf-roche dgkf-roche requested a review from maksymiuks June 18, 2025 19:48
Copy link
Collaborator

@maksymiuks maksymiuks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool! It will definitely simplify the graph and make it clearer. Thank you for that!

@dgkf-roche dgkf-roche merged commit e1f61d9 into dev Jun 20, 2025
@dgkf-roche dgkf-roche deleted the dev-rm-edge-relation-attr branch June 20, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments