Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/python/openproblems/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@

* `check_config`: Skip the Nextflow resource label check for components whose script is itself a Nextflow workflow. Viash renders those as a workflow rather than a process, so the labels would have no effect.

## BUG FIXES

* `read_task_metadata`: Order the task graph topologically instead of by a breadth-first search from a single root.
Tasks with more than one raw dataset no longer strand all but the first at the end of the README,
and a component is never documented before the files it consumes.

* `render_component_spec`: Include non-file arguments (e.g. `--seed`) in the arguments table,
and fall back to an argument's `description` when it has no `summary`.

# openproblems core Python v0.1.1

## NEW FUNCTIONALITY
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
import glob
import os
import re
import warnings
from collections import deque


def read_task_metadata(path: str) -> dict:
"""Read all API files in a task directory and return structured metadata.

Scans ``path`` recursively for ``comp_*.yaml`` and ``file_*.yaml`` files,
builds a directed task graph, and runs a BFS to determine render order.
builds a directed task graph, and topologically sorts it to determine
render order.

Args:
path: Path to the task directory (or ``api/`` subdirectory). A
Expand All @@ -25,8 +25,9 @@ def read_task_metadata(path: str) -> dict:
* ``file_info`` / ``comp_info`` – flat lists of info dicts
* ``file_expected_format`` / ``comp_args`` – flat lists
* ``task_graph`` – ``networkx.DiGraph``
* ``task_graph_root`` – name of the root node
* ``task_graph_order`` – BFS-ordered list of node names
* ``task_graph_roots`` – names of the nodes without any inputs
* ``task_graph_root`` – name of the first root node
* ``task_graph_order`` – topologically ordered list of node names
"""
from .. import find_project_root
from .read_task_config import read_task_config
Expand Down Expand Up @@ -62,8 +63,8 @@ def read_task_metadata(path: str) -> dict:
}

task_graph = _build_graph(files, comps)
task_graph_root = _get_root(task_graph)
task_graph_order = _bfs_order(task_graph, task_graph_root)
task_graph_roots = _get_roots(task_graph)
task_graph_order = _topological_order(task_graph, task_graph_roots)

comp_info = [c["info"] for c in comps.values()]
comp_args = [arg for c in comps.values() for arg in c["args"]]
Expand All @@ -82,7 +83,8 @@ def read_task_metadata(path: str) -> dict:
"comp_info": comp_info,
"comp_args": comp_args,
"task_graph": task_graph,
"task_graph_root": task_graph_root,
"task_graph_roots": task_graph_roots,
"task_graph_root": task_graph_roots[0] if task_graph_roots else None,
"task_graph_order": task_graph_order,
}

Expand Down Expand Up @@ -114,32 +116,32 @@ def _build_graph(files: dict, comps: dict):
return G


def _get_root(G) -> str:
def _get_roots(G) -> list[str]:
"""Nodes without inputs, i.e. the raw datasets a task starts from."""
roots = [n for n, d in G.in_degree() if d == 0]
if not roots:
return next(iter(G.nodes()))
if len(roots) > 1:
warnings.warn(
f"Multiple root nodes with in-degree 0: {roots}. Using first.",
stacklevel=4,
)
return roots[0]


def _bfs_order(G, root: str) -> list[str]:
"""BFS from root; unreachable nodes are appended afterwards (mirrors igraph)."""
visited: list[str] = []
seen: set[str] = set()
queue: deque[str] = deque([root])
return roots if roots else list(G.nodes())[:1]


def _topological_order(G, roots: list[str]) -> list[str]:
"""Order the graph so every node comes after the nodes it consumes.

Kahn's algorithm with a FIFO queue seeded with *all* roots, so a task with
several raw datasets keeps them together at the start instead of stranding
all but the first at the end. Nodes in a cycle are appended afterwards.
"""
pending = {n: d for n, d in G.in_degree()}
order: list[str] = []
seen: set[str] = set(roots)
queue: deque[str] = deque(roots)
while queue:
node = queue.popleft()
if node not in seen:
seen.add(node)
visited.append(node)
for nbr in G.successors(node):
if nbr not in seen:
queue.append(nbr)
order.append(node)
for nbr in G.successors(node):
pending[nbr] -= 1
if pending[nbr] <= 0 and nbr not in seen:
seen.add(nbr)
queue.append(nbr)
for node in G.nodes():
if node not in seen:
visited.append(node)
return visited
order.append(node)
return order
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,29 @@ def render_component_spec(spec: dict | str) -> str:
def _format_arguments(args: list[dict]) -> str:
from ._markdown import format_markdown_table

file_args = [a for a in args if a.get("type") == "file"]
if not file_args:
if not args:
return ""

rows = []
for arg in file_args:
for arg in args:
tags = []
if not arg.get("required", True):
tags.append("Optional")
if arg.get("direction") == "output":
tags.append("Output")
tag_str = f"(_{', '.join(tags)}_) " if tags else ""

summary = re.sub(r" *\n *", " ", (arg.get("summary") or "").strip()).rstrip(".")
# file arguments carry a summary via __merge__, plain ones a description
text = arg.get("summary") or arg.get("description") or ""
text = re.sub(r" *\n *", " ", text.strip()).rstrip(".")
default = arg.get("default")
default_str = f" Default: `{default}`." if default is not None else ""

rows.append(
[
f"`--{arg['arg_name']}`",
f"`{arg.get('type', '')}`",
f"{tag_str}{summary}.{default_str}",
f"{tag_str}{text}.{default_str}",
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ arguments:
__merge__: file_solution.yaml
direction: output
required: true
- name: "--seed"
type: integer
default: 1
description: "The seed for determining the train/test split."
test_resources:
- path: /resources_test/common/cxg_mouse_pancreas_atlas
dest: resources_test/common/cxg_mouse_pancreas_atlas
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,66 @@ def test_render_task_readme_qmd_from_path():

result = render_task_readme_qmd(EXAMPLE_PROJECT)
assert "## API" in result


def test_task_graph_order_is_topological(task_metadata):
G = task_metadata["task_graph"]
order = task_metadata["task_graph_order"]

assert sorted(order) == sorted(G.nodes)
for node in order:
for pred in G.predecessors(node):
msg = f"{node} is rendered before its input {pred}"
assert order.index(pred) < order.index(node), msg


def test_task_graph_order_keeps_multiple_roots_up_front():
import networkx as nx
from openproblems.project.docs.read_task_metadata import (
_get_roots,
_topological_order,
)

# a multimodal task: two raw datasets feeding a single processor
G = nx.DiGraph()
G.add_edges_from(
[
("file_mod1", "comp_process"),
("file_mod2", "comp_process"),
("comp_process", "file_train"),
]
)

roots = _get_roots(G)
order = _topological_order(G, roots)

assert roots == ["file_mod1", "file_mod2"]
assert order == ["file_mod1", "file_mod2", "comp_process", "file_train"]


def test_task_graph_order_includes_cyclic_nodes():
import networkx as nx
from openproblems.project.docs.read_task_metadata import (
_get_roots,
_topological_order,
)

G = nx.DiGraph()
G.add_edges_from([("a", "b"), ("b", "c"), ("c", "b")])

order = _topological_order(G, _get_roots(G))

assert sorted(order) == ["a", "b", "c"]
assert order[0] == "a"


def test_render_component_spec_non_file_arguments(task_metadata):
from openproblems.project.docs import render_component_spec

result = render_component_spec(task_metadata["comps"]["comp_data_processor"])

# non-file arguments are part of the API too, and describe themselves
# through `description` rather than the `summary` a __merge__ pulls in
assert "`--seed`" in result
assert "The seed for determining the train/test split" in result
assert "Default: `1`" in result
13 changes: 13 additions & 0 deletions packages/r/openproblems.docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
# openproblems.docs R v0.2.0

## DEPRECATIONS

* All exported functions are deprecated and will be removed in a future release.
`common/scripts/create_task_readme` runs the Python implementation, so
`openproblems.project.docs` in the Python `openproblems` package is the source
of truth for task documentation. `render_json_schema_example` has no
replacement there.

Note that the two README rendering bugs fixed in Python core v0.2.0
(task graph ordering and missing non-file arguments) were not backported here.

# openproblems.docs R v0.1.0

Initial release
Expand Down
3 changes: 2 additions & 1 deletion packages/r/openproblems.docs/DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ Authors@R: c(
role = c("aut"),
comment = c(ORCID = "0009-0003-8555-1361")
))
Description: OpenProblems Documentation R helper functions.
Description: OpenProblems Documentation R helper functions. Deprecated: superseded
by the `openproblems.project.docs` module of the Python `openproblems` package.
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
Expand Down
28 changes: 28 additions & 0 deletions packages/r/openproblems.docs/R/deprecated.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#' Warn that a documentation helper is deprecated
#'
#' The README rendering pipeline (`common/scripts/create_task_readme`) runs the
#' Python implementation, so this package is no longer the source of truth.
#'
#' @param what Name of the deprecated function
#' @param replacement Name of the Python function that supersedes it, or `NULL`
#' if there is none
#'
#' @noRd
.deprecate_docs <- function(what, replacement = what) {
advice <-
if (is.null(replacement)) {
"It has no replacement in the Python `openproblems` package."
} else {
paste0("Use `openproblems.project.docs.", replacement, "()` from the Python `openproblems` package instead.")
}

rlang::warn(
c(
paste0("`", what, "()` is deprecated and will be removed in a future release."),
i = advice
),
class = "deprecatedWarning",
.frequency = "once",
.frequency_id = paste0("openproblems.docs::", what)
)
}
6 changes: 6 additions & 0 deletions packages/r/openproblems.docs/R/openproblems.docs-package.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
#' @keywords internal
#'
#' @section Deprecated:
#' This package is deprecated and will be removed in a future release. The
#' README rendering pipeline (`common/scripts/create_task_readme`) runs the
#' Python `openproblems` package, so `openproblems.project.docs` is the source
#' of truth for task documentation.
"_PACKAGE"

## usethis namespace: start
Expand Down
6 changes: 6 additions & 0 deletions packages/r/openproblems.docs/R/read_component_spec.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#' @param path Path to a component spec yaml, usually in `src/api/comp_*.yaml`
#' @return A list with compontent info and arguments
#'
#' @section Deprecated:
#' Superseded by `openproblems.project.docs.read_component_spec()` in the Python
#' `openproblems` package, which is what `create_task_readme` runs.
#'
#' @export
#' @examples
#' path <- system.file(
Expand All @@ -14,6 +18,8 @@
#'
#' read_component_spec(path)
read_component_spec <- function(path) {
.deprecate_docs("read_component_spec")

data <- openproblems::read_nested_yaml(path)

tryCatch(
Expand Down
6 changes: 6 additions & 0 deletions packages/r/openproblems.docs/R/read_file_format.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#' @param path Path to a file format yaml, usually in `src/api/file_*.yaml`
#' @return A list with file format info and expected_format
#'
#' @section Deprecated:
#' Superseded by `openproblems.project.docs.read_file_format()` in the Python
#' `openproblems` package, which is what `create_task_readme` runs.
#'
#' @export
#' @examples
#' path <- system.file(
Expand All @@ -14,6 +18,8 @@
#'
#' read_file_format(path)
read_file_format <- function(path) {
.deprecate_docs("read_file_format")

data <- openproblems::read_nested_yaml(path)

tryCatch(
Expand Down
6 changes: 6 additions & 0 deletions packages/r/openproblems.docs/R/read_task_config.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
#'
#' @param path Path to a project config file
#'
#' @section Deprecated:
#' Superseded by `openproblems.project.docs.read_task_config()` in the Python
#' `openproblems` package, which is what `create_task_readme` runs.
#'
#' @importFrom cli cli_inform
#' @importFrom openproblems.utils validate_object
#'
Expand All @@ -15,6 +19,8 @@
#'
#' read_task_config(path)
read_task_config <- function(path) {
.deprecate_docs("read_task_config")

proj_conf <- openproblems::read_nested_yaml(path)

tryCatch(
Expand Down
6 changes: 6 additions & 0 deletions packages/r/openproblems.docs/R/read_task_metadata.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#' @param path Path to the API directory of a task
#' @return A list with the api info
#'
#' @section Deprecated:
#' Superseded by `openproblems.project.docs.read_task_metadata()` in the Python
#' `openproblems` package, which is what `create_task_readme` runs.
#'
#' @importFrom cli cli_inform cli_abort
#'
#' @export
Expand All @@ -15,6 +19,8 @@
#'
#' task_metadata
read_task_metadata <- function(path) {
.deprecate_docs("read_task_metadata")

cli::cli_inform(paste0("Looking for project root in '", path, "'"))
project_path <- openproblems::find_project_root(path)
if (is.null(project_path)) {
Expand Down
Loading