Skip to content

Conversation

@seneschall
Copy link

@seneschall seneschall commented Dec 23, 2025

This adds a minimal wrapper for MOFA2 under bio/mofa2 which trains a model with default values and writes the model to an HDF5 file.

QC

While the contributions guidelines are more extensive, please particularly ensure that:

  • test.py was updated to call any added or updated example rules in a Snakefile
  • input: and output: file paths in the rules can be chosen arbitrarily
  • wherever possible, command line arguments are inferred and set automatically (e.g. based on file extensions in input: or output:)
  • temporary files are either written to a unique hidden folder in the working directory, or (better) stored where the Python function tempfile.gettempdir() points to
  • the meta.yaml contains a link to the documentation of the respective tool or command under url:
  • conda environments use a minimal amount of channels and packages, in recommended ordering

Summary by CodeRabbit

  • New Features

    • Added MOFA2 workflow for training multi-omics factor models from parquet inputs, producing HDF5-trained models.
    • Configurable scaling options for groups and views.
  • Tests

    • Added test coverage for the MOFA2 workflow and updated alignment test formatting.
  • Chores / Packaging

    • Added reproducible environment manifests for linux-64 and an environment YAML.
    • Added repository attributes to improve merge and language handling.
  • Documentation

    • Added package metadata and usage notes for the MOFA2 wrapper.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 23, 2025

📝 Walkthrough

Walkthrough

Adds a new MOFA2 wrapper: conda environment definitions (YAML and pinned), package metadata, an R wrapper that trains MOFA2 from Parquet, a Snakemake test rule, .gitattributes, and an accompanying test addition.

Changes

Cohort / File(s) Summary
Git configuration
\.gitattributes
Adds attributes marking pixi.lock as binary and setting linguist language/metadata to prevent diffs/three-way merges.
Conda environments
bio/mofa2/environment.yaml, bio/mofa2/environment.linux-64.pin.txt
Adds environment YAML declaring channels and dependencies (bioconductor-mofa2, mofapy2, r-arrow, etc.) and an @EXPLICIT linux-64 pin file with exact package URLs and hashes.
Package metadata
bio/mofa2/meta.yaml
New meta describing the MOFA2 wrapper: package info, input schema (parquet with sample, feature, view, value, optional group), output (HDF5), and scaling options (scale_group/scale_views).
Wrapper & test rule
bio/mofa2/wrapper.R, bio/mofa2/test/Snakefile
Adds R wrapper that reads Parquet, configures/prepares and trains a MOFA2 model (honors scale_groups/scale_views params), and a Snakemake rule mapping {data}.parquet -> {data}.hdf5 with logging.
Test updates
test_wrappers.py
Minor formatting tweaks and a new test_mofa2(run) test to invoke the added wrapper via Snakemake.

Sequence Diagram(s)

sequenceDiagram
    participant Snakemake as Snakemake Rule
    participant Wrapper as wrapper.R
    participant MOFA2 as MOFA2 Library
    participant Input as Parquet Input
    participant Output as HDF5 Output

    Snakemake->>Wrapper: Execute with params (scale_groups, scale_views), input/output paths
    Wrapper->>Input: Read parquet (sample, feature, view, group, value)
    Wrapper->>MOFA2: Load libs and create MOFA object
    Wrapper->>MOFA2: Apply scaling options and prepare model
    Wrapper->>MOFA2: Train model (run_mofa)
    MOFA2->>Output: Write trained model to HDF5
    Output-->>Snakemake: HDF5 produced
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: added wrapper for MOFA2' clearly and concisely summarizes the main change: adding a new MOFA2 wrapper following conventional commit style.
Description check ✅ Passed The PR description covers the main purpose, includes the required QC checklist with all items marked complete, and demonstrates that contribution guidelines were followed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af345bf and da1b6b0.

📒 Files selected for processing (1)
  • test_wrappers.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • test_wrappers.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: docs
  • GitHub Check: testing
  • GitHub Check: Summary

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (4)
.gitattributes (1)

1-2: Minor inconsistency with subdirectory .gitattributes

The root .gitattributes is missing the -diff flag that's present in bio/mofa2/.gitattributes. While not critical, it's worth checking whether both should consistently prevent diff generation for pixi.lock files.

Root (Line 2): pixi.lock merge=binary linguist-language=YAML linguist-generated=true
Bio/mofa2 (Line 2): pixi.lock merge=binary linguist-language=YAML linguist-generated=true -diff

bio/mofa2/meta.yaml (1)

15-15: Consider rephrasing the group parameter documentation

The current phrasing "Discouraged for beginners" might be overly cautionary. Consider rephrasing to be more informative while still indicating it's an advanced feature.

🔎 Suggested rephrasing
-    `group` (optional, advanced): Discouraged for beginners. The aim of the multi-group framework is not to capture differential changes in mean levels between the groups (as for example when doing differential RNA expression). The goal is to compare the sources of variability that drive each group.
+    `group` (optional, advanced): The multi-group framework compares sources of variability across groups rather than capturing differential changes in mean levels (e.g., differential RNA expression). Consider omitting this column unless you specifically need to model multiple distinct biological groups.
bio/mofa2/wrapper.R (2)

1-1: Consider using the standard portable shebang.

The current shebang #!/bin/R assumes R is installed at /bin/R, which may not be true on all systems. The more portable form is #!/usr/bin/env Rscript.

🔎 Proposed fix
-#!/bin/R
+#!/usr/bin/env Rscript

35-51: Consider simplifying parameter handling with as.logical().

The current implementation uses separate if statements to convert string parameters to boolean values. This works correctly but is verbose. R's as.logical() function can convert "TRUE"/"FALSE" strings directly to boolean values.

🔎 Proposed refactor
-if ("scale_groups" %in% names(snakemake@params)) {
-  if (snakemake@params[["scale_groups"]] == "FALSE") {
-    data_opts$scale_groups <- FALSE
-  }
-  if (snakemake@params[["scale_groups"]] == "TRUE") {
-    data_opts$scale_groups <- TRUE
-  }
-}
+if ("scale_groups" %in% names(snakemake@params)) {
+  data_opts$scale_groups <- as.logical(snakemake@params[["scale_groups"]])
+}

-if ("scale_views" %in% names(snakemake@params)) {
-  if (snakemake@params[["scale_views"]] == "FALSE") {
-    data_opts$scale_views <- FALSE
-  }
-  if (snakemake@params[["scale_views"]] == "TRUE") {
-    data_opts$scale_views <- TRUE
-  }
-}
+if ("scale_views" %in% names(snakemake@params)) {
+  data_opts$scale_views <- as.logical(snakemake@params[["scale_views"]])
+}
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1af3918 and 86efe3c.

⛔ Files ignored due to path filters (4)
  • bio/mofa2/test/data.parquet is excluded by !**/*.parquet
  • bio/mofa2/test/log/data.log is excluded by !**/*.log
  • bio/mofa2/test/log/microbiome.log is excluded by !**/*.log
  • bio/mofa2/test/microbiome.parquet is excluded by !**/*.parquet
📒 Files selected for processing (10)
  • .gitattributes
  • .gitignore
  • bio/mofa2/.gitattributes
  • bio/mofa2/.gitignore
  • bio/mofa2/environment.linux-64.pin.txt
  • bio/mofa2/environment.yaml
  • bio/mofa2/meta.yaml
  • bio/mofa2/test/Snakefile
  • bio/mofa2/wrapper.R
  • test_wrappers.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

  • test_wrappers.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.
📚 Learning: 2024-11-26T08:31:00.099Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.

Applied to files:

  • bio/mofa2/test/Snakefile
  • test_wrappers.py
📚 Learning: 2024-11-26T09:16:39.570Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.

Applied to files:

  • bio/mofa2/test/Snakefile
📚 Learning: 2025-02-11T12:24:22.592Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3648
File: bio/nanosim/simulator/environment.yaml:6-6
Timestamp: 2025-02-11T12:24:22.592Z
Learning: In the nanosim bioconda recipe, dependencies are carefully managed with specific version pins (e.g., scikit-learn ~=0.23.2) to ensure compatibility with pre-trained models. These dependencies don't need to be explicitly added to environment.yaml files when the main package is listed as a dependency, as they are handled through the bioconda recipe system.

Applied to files:

  • bio/mofa2/environment.yaml
  • bio/mofa2/environment.linux-64.pin.txt
📚 Learning: 2025-04-17T09:24:51.738Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See full guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file

Applied to files:

  • bio/mofa2/environment.yaml
📚 Learning: 2025-04-17T09:24:51.738Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file

Applied to files:

  • bio/mofa2/environment.yaml
📚 Learning: 2025-06-02T07:56:35.854Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 4159
File: bio/pyfaidx/environment.yaml:6-6
Timestamp: 2025-06-02T07:56:35.854Z
Learning: In the Snakemake-wrapper repository, conda dependency version pinning in environment.yaml files uses spaces around the equals sign (e.g., `- pyfaidx =0.8.1.4`) as the established coding standard, even though conda itself doesn't require the spaces.

Applied to files:

  • bio/mofa2/environment.yaml
  • bio/mofa2/environment.linux-64.pin.txt
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.

Applied to files:

  • test_wrappers.py
  • bio/mofa2/wrapper.R
📚 Learning: 2024-11-26T08:30:23.818Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.

Applied to files:

  • test_wrappers.py
📚 Learning: 2024-11-26T10:49:54.765Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.

Applied to files:

  • test_wrappers.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary
🔇 Additional comments (13)
bio/mofa2/.gitignore (1)

1-3: LGTM!

The pixi environment ignore pattern is standard and appropriate. Tracking only .pixi/config.toml while ignoring the rest of the directory follows best practices.

bio/mofa2/.gitattributes (1)

1-2: LGTM!

Marking pixi.lock as binary prevents merge conflicts while maintaining syntax highlighting. This is standard practice for lock files.

test_wrappers.py (2)

271-280: LGTM - Command formatting improvement

The multi-line format improves readability of the command arguments.


7177-7178: LGTM - MOFA2 test follows repository patterns

The test follows the established pattern for wrapper tests in this repository. The minimal test approach (ensuring the wrapper executes without errors) is appropriate per repository conventions.

bio/mofa2/environment.linux-64.pin.txt (1)

1-267: LGTM - Standard explicit environment specification

This explicit pin file follows conda best practices for reproducible environments. The @EXPLICIT format with URL hashes ensures that exact package versions can be reconstructed. While lengthy, this is the expected format for fully pinned environments.

bio/mofa2/meta.yaml (1)

1-26: Well-documented wrapper metadata

The metadata file provides clear documentation of the wrapper's purpose, input/output formats, and usage notes. The structured format with explicit column descriptions and parameter guidance will help users understand how to use the wrapper correctly.

bio/mofa2/environment.yaml (1)

6-10: The package versions specified are valid and currently available. Python 3.14.2 was released on December 5, 2025, MOFA2 v1.16.0 is available in Bioconda, and mofapy2 v0.7.2 is the latest release with no known security vulnerabilities. No action required.

bio/mofa2/test/Snakefile (1)

1-12: LGTM!

The rule structure is clean and follows Snakemake conventions. The wildcard pattern provides flexibility, and the inline parameter comments clearly explain when scaling should be enabled.

bio/mofa2/wrapper.R (5)

3-9: LGTM!

The library loading and conda environment setup correctly handle MOFA2's Python dependencies through reticulate.


11-16: LGTM!

The logging setup correctly redirects both standard output and messages to the Snakemake log file when provided.


18-24: LGTM!

The input loading correctly casts the path to character (defensive programming) and the comment clearly documents the expected Parquet schema.


26-30: LGTM!

The MOFA object creation and default options retrieval follow the standard MOFA2 workflow.


55-60: LGTM!

The prepare_mofa call correctly passes all configured options to finalize the MOFA object before training.

johanneskoester and others added 2 commits January 7, 2026 12:13
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @bio/mofa2/wrapper.R:
- Around line 43-44: The comment in wrapper.R referencing training params
(maxiter, convergence_mode, gpu_mode, verbose) is stale; either remove that line
or implement those parameters by adding them to the relevant function signature
(e.g., the mofa2 training wrapper function in wrapper.R), pass them through to
the internal train routine (ensuring names match: maxiter, convergence_mode,
gpu_mode, verbose), validate types/values, and update any calls/tests; if you
choose removal, simply delete the comment to avoid confusion.
- Around line 39-41: The code mistakenly assigns
snakemake@params[["scale_views"]] to data_opts$scale_groups; change the
assignment so snakemake@params[["scale_views"]] is stored in
data_opts$scale_views instead, i.e., replace the reference to
data_opts$scale_groups with data_opts$scale_views in the block that checks
"scale_views" in names(snakemake@params) to restore the intended scale_views
behavior.
- Line 52: The output path currently builds outfile using file.path(getwd(),
snakemake@output[[1]]); remove the unnecessary getwd()/file.path wrapping and
set outfile to snakemake@output[[1]] directly so it matches other R wrappers
(e.g., saveRDS(..., snakemake@output[[1]])) and lets Snakemake handle path
resolution.
🧹 Nitpick comments (1)
bio/mofa2/wrapper.R (1)

1-1: Consider using a more standard shebang.

The shebang #!/bin/R is non-standard. Consider using #!/usr/bin/env Rscript for better portability across systems, though in a Conda environment this may be less critical.

♻️ Proposed fix
-#!/bin/R
+#!/usr/bin/env Rscript
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f20ee7 and 4480162.

📒 Files selected for processing (2)
  • bio/mofa2/test/Snakefile
  • bio/mofa2/wrapper.R
🚧 Files skipped from review as they are similar to previous changes (1)
  • bio/mofa2/test/Snakefile
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.

Applied to files:

  • bio/mofa2/wrapper.R
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary
🔇 Additional comments (1)
bio/mofa2/wrapper.R (1)

7-9: Good implementation of MOFA2 workflow structure.

The wrapper correctly:

  • Connects to the Conda environment for MOFA2's Python backend via reticulate (lines 7-9)
  • Implements standard log redirection for Snakemake (lines 11-16)
  • Follows the proper MOFA2 workflow: create → configure → prepare → train (lines 26-30, 45-50, 54-58)

Please ensure the critical bug on line 40 is fixed before merging.

Also applies to: 11-16, 26-30, 45-50, 54-58

@seneschall
Copy link
Author

Thank you for your feedback. All issues should be fixed now. In manual local testing, the wrapper worked as intended.

@fgvieira
Copy link
Collaborator

fgvieira commented Jan 9, 2026

Is the mofapy2 dependency needed?

@seneschall
Copy link
Author

Is the mofapy2 dependency needed?

From what I understand, yes. The training of the model is done in Python and the R-tool MOFA2 just wraps mofapy2. It can either create its own conda Python environment with basilisk or use a provided external one but it needs mofapy2 in order to work.

See the official installation guide: https://biofam.github.io/MOFA2/installation.html

And the tutorial (section 6): https://raw.githack.com/bioFAM/MOFA2_tutorials/master/R_tutorials/getting_started_R.html

I thought it was better to use the conda environment provided by Snakemake and download mofapy2 from Bioconda rather than to let MOFA2 set up a separate environment and download it from a different source.

@fgvieira
Copy link
Collaborator

But if mofapy2 is needed then it should be a dependency for bioconductor-mofa2. Otherwise, how does bioconductor-mofa2 work without it?

@seneschall
Copy link
Author

But if mofapy2 is needed then it should be a dependency for bioconductor-mofa2. Otherwise, how does bioconductor-mofa2 work without it?

Well, as I mentioned, they say in their documentation that MOFA2 can download the mofapy2 dependency via basilisk. The function run_mofa even has a flag called use_basilisk which, if set to TRUE, will download mofapy2 via basilisk. This is the approach they recommend in the tutorial I linked earlier.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
bio/mofa2/environment.yaml (1)

5-8: Consider removing version pins from dependencies.

Per the snakemake-wrappers guidelines, only the main software package should have an exact version pin, and version constraints for dependencies should be avoided unless absolutely necessary.

  • bioconductor-mofa2 =1.16.0 — main package, version pin is appropriate ✓
  • r-arrow =22.0.0 and mofapy2 =0.7.2 — these are dependencies; pinning may cause unnecessary solver conflicts during future updates

The explicit inclusion of mofapy2 is justified (avoids basilisk creating a separate Python environment), but the version pin could be relaxed.

Suggested change
 dependencies:
   - bioconductor-mofa2 =1.16.0
-  - r-arrow =22.0.0
-  - mofapy2 =0.7.2
+  - r-arrow
+  - mofapy2

Based on learnings from this repository's environment.yaml conventions.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8072bd5 and af345bf.

📒 Files selected for processing (2)
  • bio/mofa2/environment.linux-64.pin.txt
  • bio/mofa2/environment.yaml
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
📚 Learning: 2025-02-11T12:24:22.592Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3648
File: bio/nanosim/simulator/environment.yaml:6-6
Timestamp: 2025-02-11T12:24:22.592Z
Learning: In the nanosim bioconda recipe, dependencies are carefully managed with specific version pins (e.g., scikit-learn ~=0.23.2) to ensure compatibility with pre-trained models. These dependencies don't need to be explicitly added to environment.yaml files when the main package is listed as a dependency, as they are handled through the bioconda recipe system.

Applied to files:

  • bio/mofa2/environment.yaml
  • bio/mofa2/environment.linux-64.pin.txt
📚 Learning: 2025-04-17T09:24:51.738Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See full guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file

Applied to files:

  • bio/mofa2/environment.yaml
📚 Learning: 2025-04-17T09:24:51.738Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file

Applied to files:

  • bio/mofa2/environment.yaml
📚 Learning: 2025-06-02T07:56:35.854Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 4159
File: bio/pyfaidx/environment.yaml:6-6
Timestamp: 2025-06-02T07:56:35.854Z
Learning: In the Snakemake-wrapper repository, conda dependency version pinning in environment.yaml files uses spaces around the equals sign (e.g., `- pyfaidx =0.8.1.4`) as the established coding standard, even though conda itself doesn't require the spaces.

Applied to files:

  • bio/mofa2/environment.yaml
  • bio/mofa2/environment.linux-64.pin.txt
📚 Learning: 2024-10-31T10:01:29.976Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3374
File: utils/datavzrd/environment.yaml:7-9
Timestamp: 2024-10-31T10:01:29.976Z
Learning: In the `utils/datavzrd` wrapper, numpy, pandas, and polars are required dependencies because users can pass objects of these types via `params`, and since the wrapper evaluates them via YTE, these packages need to be present in the environment to support all potential params.

Applied to files:

  • bio/mofa2/environment.yaml
📚 Learning: 2024-11-26T08:30:23.818Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.

Applied to files:

  • bio/mofa2/environment.yaml
📚 Learning: 2025-02-11T12:24:22.592Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3648
File: bio/nanosim/simulator/environment.yaml:6-6
Timestamp: 2025-02-11T12:24:22.592Z
Learning: For bioconda packages like nanosim, dependencies like scikit-learn are managed through the bioconda recipe and don't need to be explicitly added to the environment.yaml file when the main package is listed as a dependency.

Applied to files:

  • bio/mofa2/environment.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary
🔇 Additional comments (2)
bio/mofa2/environment.yaml (1)

1-4: LGTM - Channels are correctly configured.

The channel order (conda-forge, bioconda, nodefaults) follows the recommended convention for Bioconda packages.

bio/mofa2/environment.linux-64.pin.txt (1)

1-268: LGTM - Standard explicit pin file for reproducibility.

This auto-generated pin file correctly captures the resolved environment for linux-64, enabling reproducible builds. The key package versions align with environment.yaml specifications:

  • bioconductor-mofa2-1.16.0 (line 268)
  • mofapy2-0.7.2 (line 178)
  • r-arrow-22.0.0 (line 257)

@fgvieira
Copy link
Collaborator

Any reason you keep the .gitignore file?

@seneschall
Copy link
Author

seneschall commented Jan 13, 2026

Any reason you keep the .gitignore file?

At first I deleted it because I thought it had been created entirely by my pixi development environment but then I noticed that there was also a .gitignore with different contents on the main branch (at least my local clone) so I thought that was supposed to be there and I copied the contents of that over to the mofa2-wrapper branch. If that was a misunderstanding on my part, I will delete it of course!

EDIT: It's also on the main branch here on Github.

@fgvieira
Copy link
Collaborator

Sorry! I mean the .gitattributes! 😄

@seneschall
Copy link
Author

seneschall commented Jan 13, 2026

Oh sorry, I must have missed it. It seems I only deleted bio/mofa2/.gitattributes! I've removed it now.

@fgvieira
Copy link
Collaborator

It looks good to me!
Lets just wait for @johanneskoester in case he has more comments.

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.

3 participants