Conversation
I tasked Cursor with synchronizing the documentation across ModelArray, ModelArrayIO, and ConFixel.
There was a problem hiding this comment.
Pull request overview
This PR updates ModelArray documentation to reflect the migration from the legacy ConFixel tooling to the newer ModelArrayIO companion package, aligning terminology and CLI references across vignettes and READMEs.
Changes:
- Replace ConFixel/ConVoxel/ConCifti references with ModelArrayIO and corresponding CLI commands (e.g.,
confixel,convoxel,fixelstats_write,ciftistats_write,volumestats_write). - Update installation and container documentation to describe ModelArrayIO as the companion Python package.
- Refresh README/README.Rmd overview text to point users to ModelArrayIO and note ConFixel is superseded.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/wrap_function.Rmd | Updates CIFTI write-back step wording to ciftistats_write / ModelArrayIO. |
| vignettes/walkthrough.Rmd | Updates walkthrough narrative and commands to ModelArrayIO-based fixel workflow. |
| vignettes/voxel-wise_data.Rmd | Updates voxel-wise conversion guidance to ModelArrayIO (convoxel, volumestats_write). |
| vignettes/installations.Rmd | Updates installation instructions to install ModelArrayIO (and newer Python). |
| vignettes/doc_for_developer.Rmd | Renames Docker build section to reference ModelArrayIO. |
| vignettes/container.Rmd | Updates container vignette language to ModelArrayIO-focused workflows. |
| README.md | Updates public-facing overview and install/container text to ModelArrayIO. |
| README.Rmd | Same as README.md source (regenerates README.md). |
Comments suppressed due to low confidence (1)
vignettes/wrap_function.Rmd:289
- The updated text refers to ModelArrayIO, but the example paths still use a
ConCifti/directory name. If this directory name is legacy from older ConCifti/ConFixel docs, consider renaming it (or adding a short note that it's just an arbitrary local folder name) to avoid confusing readers.
We now use `ciftistats_write` (from ModelArrayIO) to convert the results into dscalar.nii files:
```{.console}
$ ciftistats_write \
--cohort-file ${HOME}/Desktop/myCustomFunction/ConCifti/study-HBN_desc-participants_thickness.csv \
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `/path/to/singularity/image/modelarray_confixel_<tagName>.sif` is the full path to the singularity image (`.sif`) you pulled; | ||
|
|
||
| `ModelArray` and `ConFixel` are both included in this container. To run `ConFixel`, simply replace `<command you want to run>` with `ConFixel` commands (e.g., `confixel`). | ||
| `ModelArray` and ModelArrayIO (Python CLIs such as `confixel`, `convoxel`, `fixelstats_write`) are both included in this container. Replace `<command you want to run>` with the CLI you need (e.g., `confixel --help`). |
There was a problem hiding this comment.
The vignette claims ModelArrayIO CLIs (e.g. confixel, convoxel, fixelstats_write) are included in this container. The container definitions in this repo (Dockerfile/Apptainer.def) don’t install ModelArrayIO, so these commands likely won’t exist at runtime. Please either install ModelArrayIO in the image build or update this sentence to avoid a broken example.
| `ModelArray` and ModelArrayIO (Python CLIs such as `confixel`, `convoxel`, `fixelstats_write`) are both included in this container. Replace `<command you want to run>` with the CLI you need (e.g., `confixel --help`). | |
| `ModelArray` is included in this container. If you also need ModelArrayIO Python CLIs (such as `confixel`, `convoxel`, `fixelstats_write`), install them separately on your system or use a container image that bundles ModelArrayIO. Replace `<command you want to run>` with the CLI you need. |
There was a problem hiding this comment.
@mattcieslak do we want the Docker container to include both ModelArray and ModelArrayIO? If so, then I'll ignore these suggestions and we can work on adding ModelArray to the container.
| --- | ||
|
|
||
| This page walks through the installation of `ModelArray` package and necessary dependent packages/libraries. If you have difficulty installing necessary packages (e.g., on HPC clusters), you also have an option to use the container image of `ModelArray + ConFixel`. Please refer to `vignette("container")` for more. | ||
| This page walks through the installation of `ModelArray` package and necessary dependent packages/libraries. If you have difficulty installing necessary packages (e.g., on HPC clusters), you also have an option to use the container image of `ModelArray` and **ModelArrayIO** (the Docker image is still named `pennlinc/modelarray_confixel` for historical reasons). Please refer to `vignette("container")` for more. |
There was a problem hiding this comment.
This paragraph suggests users can use a container image that includes both ModelArray and ModelArrayIO. The container build artifacts in this repo (Dockerfile/Apptainer.def) currently only install ModelArray and do not install ModelArrayIO. Please either update the container build to include ModelArrayIO or qualify the text so users don’t expect the Python CLIs to be present.
| This page walks through the installation of `ModelArray` package and necessary dependent packages/libraries. If you have difficulty installing necessary packages (e.g., on HPC clusters), you also have an option to use the container image of `ModelArray` and **ModelArrayIO** (the Docker image is still named `pennlinc/modelarray_confixel` for historical reasons). Please refer to `vignette("container")` for more. | |
| This page walks through the installation of `ModelArray` package and necessary dependent packages/libraries. If you have difficulty installing necessary packages (e.g., on HPC clusters), you also have an option to use the container image of `ModelArray` (the Docker image is still named `pennlinc/modelarray_confixel` for historical reasons). Note that this container image does **not** include the companion Python package **ModelArrayIO** or its command-line tools (such as `confixel` and `convoxel`); please refer to `vignette("container")` and the sections below for installing and using ModelArrayIO separately. |
| For more details on how to write unit tests + test out, please check out `Testing` chapters in the book ["R Packages" written by Hadley Wickham, Jennifer Bryan](https://r-pkgs.org/index.html) | ||
|
|
||
| ## Building the Docker image for ModelArray + ConFixel | ||
| ## Building the Docker image for ModelArray + ModelArrayIO |
There was a problem hiding this comment.
This header says the repo’s Docker image is for “ModelArray + ModelArrayIO”, but the Dockerfile in the repo only installs the ModelArray R package (no ModelArrayIO install step). Please either add ModelArrayIO installation to the Dockerfile or rename this section to match what the Docker image actually contains.
| ## Building the Docker image for ModelArray + ModelArrayIO | |
| ## Building the Docker image for ModelArray |
| includes `ModelArray` and `ModelArrayIO` (image name remains | ||
| `modelarray_confixel` for backward compatibility). With this container | ||
| image, there is no need for the user to install `ModelArray`, | ||
| `ModelArrayIO`, and dependent R and Python packages manually. Please see |
There was a problem hiding this comment.
This claims the container image includes ModelArrayIO, but the Dockerfile/Apptainer definition in this repo only installs the ModelArray R package and doesn’t install ModelArrayIO. Please either update the image build to include ModelArrayIO or adjust this text so users don’t expect the Python CLIs to be present.
| includes `ModelArray` and `ModelArrayIO` (image name remains | |
| `modelarray_confixel` for backward compatibility). With this container | |
| image, there is no need for the user to install `ModelArray`, | |
| `ModelArrayIO`, and dependent R and Python packages manually. Please see | |
| includes `ModelArray` and its R dependencies (image name remains | |
| `modelarray_confixel` for backward compatibility). With this container | |
| image, there is no need for the user to install `ModelArray` and its | |
| dependent R packages manually. Please see |
| What is covered on this page? How to use the Docker/Singularity image `pennlinc/modelarray_confixel`, which bundles `ModelArray` and ModelArrayIO (the image name is historical). | ||
|
|
||
| What is not covered on this page? Step by step details on how to run `ModelArray` and `ConFixel`. For a full walkthrough on how to use our software, please refer to `vignette("walkthrough")`. We highly suggest reviewing that page if you're new to `ModelArray`. | ||
| What is not covered on this page? Step by step details on how to run `ModelArray` and ModelArrayIO. For a full walkthrough on how to use our software, please refer to `vignette("walkthrough")`. We highly suggest reviewing that page if you're new to `ModelArray`. |
There was a problem hiding this comment.
This section states the pennlinc/modelarray_confixel image “bundles ModelArray and ModelArrayIO”. However, the Dockerfile/Apptainer definition in this repo only installs the R package and does not install ModelArrayIO (or any Python tooling). Please either update the container build to install ModelArrayIO, or adjust this wording to reflect what’s actually included in the image built from this repository.
| * Install `ModelArray` from GitHub - see [this section](https://pennlinc.github.io/ModelArray/articles/installations.html#install-modelarray-r-package-from-github) | ||
|
|
||
| Additionally, we also provide a [container image](https://hub.docker.com/r/pennlinc/modelarray_confixel) that includes `ModelArray` and `ConFixel`. With this container image, there is no need for the user to install `ModelArray`, `ConFixel`, and dependent R and Python packages. Please see [this webpage](https://pennlinc.github.io/ModelArray/articles/container.html) for how to use this container image. | ||
| Additionally, we also provide a [container image](https://hub.docker.com/r/pennlinc/modelarray_confixel) that includes `ModelArray` and **ModelArrayIO** (image name remains `modelarray_confixel` for backward compatibility). With this container image, there is no need for the user to install `ModelArray`, `ModelArrayIO`, and dependent R and Python packages manually. Please see [this webpage](https://pennlinc.github.io/ModelArray/articles/container.html) for how to use this container image. |
There was a problem hiding this comment.
README says the Docker container image includes ModelArrayIO, but the Dockerfile/Apptainer definition in this repository only installs the R package and does not install ModelArrayIO. Please either update the container build to include ModelArrayIO, or revise this sentence to avoid setting incorrect expectations about what’s inside the image built from this repo.
| Additionally, we also provide a [container image](https://hub.docker.com/r/pennlinc/modelarray_confixel) that includes `ModelArray` and **ModelArrayIO** (image name remains `modelarray_confixel` for backward compatibility). With this container image, there is no need for the user to install `ModelArray`, `ModelArrayIO`, and dependent R and Python packages manually. Please see [this webpage](https://pennlinc.github.io/ModelArray/articles/container.html) for how to use this container image. | |
| Additionally, we also provide a [container image](https://hub.docker.com/r/pennlinc/modelarray_confixel) that includes the `ModelArray` R package (image name remains `modelarray_confixel` for backward compatibility). With this container image, there is no need for the user to install `ModelArray` and its dependent R packages manually. For guidance on installing and using the companion Python package **ModelArrayIO**, please refer to the [Installation](https://pennlinc.github.io/ModelArray/articles/installations.html) and [container](https://pennlinc.github.io/ModelArray/articles/container.html) documentation pages. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
I tasked Cursor with synchronizing the documentation across ModelArray, ModelArrayIO, and ConFixel.
Related to PennLINC/ModelArrayIO#8 and PennLINC/ConFixel#43.