NodeBlock Docs CI by tamsinrogers · Pull Request #2196 · FCP-INDI/C-PAC

tamsinrogers · 2025-03-13T19:03:04Z

Description

Adds continuous integration functionality for C-PAC documentation. The nodeblock_index.json file is populated with updated nodeblock documentation, which will then be read into https://github.com/FCP-INDI/cpac-docs.

Technical details

Once merged, the C-PAC repo will also need the CPAC_DOC_REPO_ACCESS secret added.

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the develop branch of the repository.
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I updated the changelog.
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

shnizzedy

I'm all for more CI!

Is https://github.com/FCP-INDI/cpac-docs the future replacement for the main C-PAC docs repo (https://github.com/FCP-INDI/fcp-indi.github.io) or is it a component repo like https://github.com/FCP-INDI/C-PAC_tutorials ? I can't tell from that repo / this PR if it's already in use or just in preparation for future use.

I think all this functionality is just in service of the documentation CI, not anything happening in C-PAC itself. In that case, I think we should move the scripts from scripts to .github/scripts and add

.github/scripts/config_extractor.py
.github/scripts/nodeblock_docs.py

to .dockerignore after https://github.com/childmindresearch/C-PAC-test/blob/9c876c3a29a0ca97d4acfa7d6cfbd037f5ba9e44/.dockerignore#L7

OR these scripts and most of the workflow could live in https://github.com/FCP-INDI/cpac-docs and this repo could just add a workflow that triggers the workflow over there.

Not at all a blocker for merging this, but I think this would be easier to follow and maintain with more thorough type hints.

There's some wheel reinventing here that I'm not sure is necessary (but it might be? I'm also not sure it isn't necessary). I commented some suggestions, that, if accepted, I think several of the functions here could then be removed entirely.

shnizzedy · 2025-03-14T16:24:33Z

+    import sys
+
+    sys.path.append(".")
+
+    CPAC_DIR = pl.Path(".")
+    CONFIG_DIR = pl.Path("configs")
+
+    fetch_and_expand_all_cpac_configs(CPAC_DIR, CONFIG_DIR)
+
+    with open("nodeblock_index.json") as f:
+        nbs = json.load(f)
+
+    configs = {}
+
+    for config_path in CONFIG_DIR.glob("*.yml"):
+        with open(config_path, "r", encoding="utf-8") as handle:
+            config = yaml.safe_load(handle)
+
+        configs[config_path.stem] = config


We can use existing infrastructure to load all the preconfigs

Suggested change

import sys

sys.path.append(".")

CPAC_DIR = pl.Path(".")

CONFIG_DIR = pl.Path("configs")

fetch_and_expand_all_cpac_configs(CPAC_DIR, CONFIG_DIR)

with open("nodeblock_index.json") as f:

nbs = json.load(f)

configs = {}

for config_path in CONFIG_DIR.glob("*.yml"):

with open(config_path, "r", encoding="utf-8") as handle:

config = yaml.safe_load(handle)

configs[config_path.stem] = config

from CPAC.pipeline import ALL_PIPELINE_CONFIGS

from CPAC.utils.configuration.configuration import Configuration, Preconfiguration

with open("nodeblock_index.json") as f:

nbs = json.load(f)

configs: dict[str, Configuration] = {config: Preconfiguration(config, skip_env_check=True) for config in ALL_PIPELINE_CONFIGS}

and then we'll have a dict[str, Configuration] instead of a dict[str, dict] so we'll continue to have access to all the built-in methods

shnizzedy · 2025-03-14T16:40:54Z

+
+        configs_with_this_enabled = []
+        for config_name, config in configs.items():
+            if _any_true_in_config(config, paths):


If I'm reading this right and paths is what's in a switch key in a NodeBlock, if we load the configurations as Configurations instead of dicts, we could use the switch_is_on method like

Suggested change

if _any_true_in_config(config, paths):

if all(config.switch_is_on(switch) for switch in paths):

and then we don't need a lot of the new infrastructure here.

was about to add the changes I made over at the prototype repo, but using the config class directly is of course better.

Just for reference:

https://github.com/childmindresearch/blockbuster/blob/bdf35bbed7c2130c1f6297bae28117a4920d075d/cpac_config_extractor.py#L120-L124

shnizzedy · 2025-03-14T16:41:32Z

+def check_cpac_config(
+    config: dict,
+) -> tuple[Literal[True], None] | tuple[Literal[False], Exception]:
+    """Checks if the specified file is a valid C-PAC config file"""
+    from CPAC.utils.configuration.configuration import Configuration  # noqa
+
+    try:
+        Configuration(config)
+    except Exception as e:
+        return False, e
+    return True, None


This isn't used anywhere?

shnizzedy · 2025-03-14T16:58:35Z

+def find_nodeblocks(root_dir):
+    visitor = NodeBlockVisitor()
+    root_path = Path(root_dir)
+
+    # Walk through all Python files
+    for python_file in root_path.rglob('*.py'):
+        try:
+            with open(python_file, 'r', encoding='utf-8') as f:
+                source = f.read()
+                visitor.current_source = source.splitlines()
+                visitor.current_file = python_file.relative_to(root_path)
+                tree = ast.parse(source)
+                visitor.visit(tree)
+        except Exception as e:
+            print(f"Error processing {python_file}: {e}")
+
+    return visitor.nodeblocks


I think this is different enough that it's probably worth having both for now, but it's pretty similar to

C-PAC/CPAC/pipeline/resource_inventory.py

Lines 64 to 108 in b3521fc

def import_nodeblock_functions(

package_name: str, exclude: Optional[list[str]] = None

) -> list[NodeBlockFunction]:

"""

Import all functions with the @nodeblock decorator from all modules and submodules in a package.

Parameters

----------

package_name

The name of the package to import from.

exclude

A list of module names to exclude from the import.

"""

if exclude is None:

exclude = []

functions: list[NodeBlockFunction] = []

package = importlib.import_module(package_name)

package_path = package.__path__[0] # Path to the package directory

for root, _, package_files in os.walk(package_path):

for file in package_files:

if file.endswith(".py") and file != "__init__.py":

# Get the module path

rel_path = os.path.relpath(os.path.join(root, file), package_path)

module_name = f"{package_name}.{rel_path[:-3].replace(os.sep, '.')}"

if module_name in exclude:

continue

# Import the module

try:

with patch.dict(

"sys.modules", {exclusion: None for exclusion in exclude}

):

module = importlib.import_module(module_name)

except (ImportError, TraitError, ValueError) as e:

UTLOGGER.debug(f"Failed to import {module_name}: {e}")

continue

# Extract nodeblock-decorated functions from the module

for _name, obj in inspect.getmembers(

module, predicate=lambda obj: isinstance(obj, NodeBlockFunction)

):

functions.append(obj)

return functions

and we could probably deduplicate at some point in the future.

hey Jon, thanks for your comments - they're super helpful in understanding how CI will work best in this context :)

In regard to

Is https://github.com/FCP-INDI/cpac-docs the future replacement for the main C-PAC docs repo (https://github.com/FCP-INDI/fcp-indi.github.io) or is it a component repo like https://github.com/FCP-INDI/C-PAC_tutorials ? I can't tell from that repo / this PR if it's already in use or just in preparation for future use.

Yes, I'm using https://github.com/FCP-INDI/cpac-docs as the future replacement for the main C-PAC docs repo. At the time of this PR, it didn't have any content - we were just using it as a place to store nodeblock_index.json.

I opened an issue in that new repo here FCP-INDI/cpac-docs#12 to address the others. Looking forward to getting it all working!

shnizzedy · 2025-06-12T17:18:16Z

+      # Copy the JSON file to the target repo and commit it
+      - name: Commit documentation to target repo
+        run: |
+          cp source-repo/nodeblock_index.json target-repo/


Suggested change

cp source-repo/nodeblock_index.json target-repo/

cp source-repo/nodeblock_index.json target-repo/src/

Now that FCP-INDI/cpac-docs#120 is merged, I think this change should address my suggestions re: FCP-INDI/cpac-docs#24

tamsinrogers · 2025-06-13T20:45:26Z

hi @shnizzedy,

I applied the above suggestions - let me know if I missed something! Since I moved the scripts from /scripts into .github/scripts, there's a chance some functionality could be off.

shnizzedy

Sorry I missed this for 2 months. Yeah, I think it looks good! Once it's merged into main, we can see how it works and patch any bugs directly then.

nx10 added 3 commits March 13, 2025 14:39

Add nodeblock docs CI

21ccb1a

Fix small typos

244c33f

Small fix

5cf7a79

tamsinrogers requested a review from a team March 13, 2025 19:03

github-project-automation Bot added this to C-PAC Development Mar 13, 2025

tamsinrogers marked this pull request as draft March 13, 2025 19:41

tamsinrogers and others added 5 commits March 13, 2025 15:46

add config extractor

1d4d021

move to correct scripts folder

dc7b8ac

path update

d16f692

add utils functionality

3906c7f

Cleanup; Small fixes

9c876c3

tamsinrogers marked this pull request as ready for review March 13, 2025 20:25

shnizzedy reviewed Mar 14, 2025

View reviewed changes

tamsinrogers had a problem deploying to ACCESS March 15, 2025 18:56 — with GitHub Actions Failure

tamsinrogers mentioned this pull request Mar 17, 2025

address CI nodeblock index functionality FCP-INDI/cpac-docs#12

Closed

shnizzedy requested changes Jun 12, 2025

View reviewed changes

tamsinrogers added 6 commits June 13, 2025 16:23

update target repo source

6402c43

remove check_cpac_config

07138ba

add to dockerignore

5615446

move scripts to github/scripts

7e331ca

use existing infrastructure to load all the preconfigs

c328450

load the configurations as Configurations instead of dicts

4400f67

shnizzedy approved these changes Aug 20, 2025

View reviewed changes

Navigation Menu

Uh oh!

Uh oh!

NodeBlock Docs CI - #2196

NodeBlock Docs CI#2196
tamsinrogers wants to merge 14 commits into
FCP-INDI:developfrom
childmindresearch:main

tamsinrogers commented Mar 13, 2025

Uh oh!

shnizzedy left a comment

Uh oh!

shnizzedy Mar 14, 2025

Uh oh!

shnizzedy Mar 14, 2025

Uh oh!

nx10 Apr 13, 2025

Uh oh!

shnizzedy Mar 14, 2025

Uh oh!

shnizzedy Mar 14, 2025

Uh oh!

tamsinrogers Mar 17, 2025

Uh oh!

shnizzedy Jun 12, 2025

Uh oh!

shnizzedy Jun 12, 2025

Uh oh!

tamsinrogers commented Jun 13, 2025

Uh oh!

shnizzedy left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if _any_true_in_config(config, paths):
	if all(config.switch_is_on(switch) for switch in paths):

	def import_nodeblock_functions(
	package_name: str, exclude: Optional[list[str]] = None
	) -> list[NodeBlockFunction]:
	"""
	Import all functions with the @nodeblock decorator from all modules and submodules in a package.

	Parameters
	----------
	package_name
	The name of the package to import from.

	exclude
	A list of module names to exclude from the import.
	"""
	if exclude is None:
	exclude = []
	functions: list[NodeBlockFunction] = []
	package = importlib.import_module(package_name)
	package_path = package.__path__[0] # Path to the package directory

	for root, _, package_files in os.walk(package_path):
	for file in package_files:
	if file.endswith(".py") and file != "__init__.py":
	# Get the module path
	rel_path = os.path.relpath(os.path.join(root, file), package_path)
	module_name = f"{package_name}.{rel_path[:-3].replace(os.sep, '.')}"
	if module_name in exclude:
	continue

	# Import the module
	try:
	with patch.dict(
	"sys.modules", {exclusion: None for exclusion in exclude}
	):
	module = importlib.import_module(module_name)
	except (ImportError, TraitError, ValueError) as e:
	UTLOGGER.debug(f"Failed to import {module_name}: {e}")
	continue
	# Extract nodeblock-decorated functions from the module
	for _name, obj in inspect.getmembers(
	module, predicate=lambda obj: isinstance(obj, NodeBlockFunction)
	):
	functions.append(obj)

	return functions

	cp source-repo/nodeblock_index.json target-repo/
	cp source-repo/nodeblock_index.json target-repo/src/

Uh oh!

Uh oh!

Conversation

tamsinrogers commented Mar 13, 2025

Description

Technical details

Checklist

Developer Certificate of Origin

Uh oh!

shnizzedy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tamsinrogers commented Jun 13, 2025

Uh oh!

shnizzedy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants