Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update r-archr to 1.0.3 #52121

Merged
merged 14 commits into from
Nov 20, 2024
Merged

Update r-archr to 1.0.3 #52121

merged 14 commits into from
Nov 20, 2024

Conversation

BiocondaBot
Copy link
Collaborator

Update r-archr: 1.0.21.0.3

install with bioconda Conda

Info Link or Description
Recipe recipes/r-archr (click to view/edit other files)
Summary This package is designed to streamline scATAC analyses in R.
Home https://www.archrproject.com
Releases https://github.com/GreenleafLab/ArchR/tags
Recipe Maintainer(s) @jdblischak, @mfansler
Author @GreenleafLab

This pull request was automatically generated (see docs).

@BiocondaBot BiocondaBot added autobump Automatic Version Update new version labels Nov 15, 2024
Copy link
Contributor

coderabbitai bot commented Nov 15, 2024

Caution

Review failed

The pull request is closed.

📝 Walkthrough
📝 Walkthrough

The pull request involves updates to the meta.yaml file for the r-archr package. The version has been incremented from 1.0.2 to 1.0.3, and the SHA256 checksum for the source tarball has been updated from afe4d82975e9d75018e9ec9fda3d116f34f99ad1d45990cbc5a2be7dea8df352 to 9c07c785a095062a998ed94c65df17a58f273d0d64062c14210f0a2c491304cf. The license has changed from GPL-2.0-or-later to MIT, with the license family updated accordingly. The r-base dependency has been modified to remove the version constraint, and several new dependencies have been added, including bioconductor-sparsematrixstats, r-chromvarmotifs, r-devtools, r-harmony, r-presto, r-rcpparmadillo, r-seurat, and r-seuratobject. These changes are reflected in both the build and run sections of the requirements. Additionally, a patch has been applied to the R scripts CreateArrow.R and MatrixGeneScores.R, which modifies the thread assignment logic to cap the number of threads based on a user-defined limit. The build.sh script has also been updated to include the --install-tests option during package installation.

Possibly related PRs

  • Update r-archr to 1.0.3 #52121: The changes in this PR update the r-archr package version from 1.0.2 to 1.0.3, which directly corresponds to the version update in the main PR.

Suggested labels

please review & merge


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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: 1

🧹 Outside diff range and nitpick comments (1)
recipes/r-archr/meta.yaml (1)

Line range hint 1-103: Scope of changes exceeds typical version bump

While this PR is marked as a version bump, it includes several significant changes:

  1. Version update from 1.0.2 to 1.0.3
  2. License change from GPL-2.0-or-later to MIT
  3. Removal of r-base version constraint
  4. Addition of 6 new dependencies

These changes, particularly the license change and dependency modifications, warrant careful review and testing.

Consider:

  1. Testing the package with different r-base versions
  2. Documenting the rationale for the license change
  3. Verifying that all new dependencies are properly versioned
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 743061e and 3aff1ca.

📒 Files selected for processing (2)
  • recipes/r-archr/meta.yaml (8 hunks)
  • recipes/r-archr/patches/0001-cap-threads.patch (0 hunks)
💤 Files with no reviewable changes (1)
  • recipes/r-archr/patches/0001-cap-threads.patch
🧰 Additional context used
🪛 yamllint
recipes/r-archr/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (3)
recipes/r-archr/meta.yaml (3)

1-1: Verify version and checksum authenticity

The version has been updated to 1.0.3 with a new SHA256 checksum.

#!/bin/bash
# Description: Verify version and checksum
# 1. Check latest release version
gh api repos/GreenleafLab/ArchR/releases/latest --jq .tag_name

# 2. Download and verify checksum
TEMP_DIR=$(mktemp -d)
cd $TEMP_DIR
curl -L https://github.com/GreenleafLab/ArchR/archive/refs/tags/v1.0.3.tar.gz -o source.tar.gz
echo "9c07c785a095062a998ed94c65df17a58f273d0d64062c14210f0a2c491304cf source.tar.gz" | sha256sum -c -

Also applies to: 11-11

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


99-99: Verify license change and file existence

The license has been changed from GPL-2.0-or-later to MIT. This is a significant change that requires verification.

#!/bin/bash
# Description: Verify license change in upstream repository
# 1. Check license in DESCRIPTION file of the latest release
curl -sL https://github.com/GreenleafLab/ArchR/archive/refs/tags/v1.0.3.tar.gz | \
  tar xz --to-stdout "*/DESCRIPTION" | \
  rg "^License:"

# 2. Check if MIT license file exists in R share directory
ls -l $PREFIX/lib/R/share/licenses/MIT 2>/dev/null || echo "MIT license file not found"

Also applies to: 101-101, 103-103


27-27: Consider maintaining r-base version constraint

The version constraint for r-base has been removed. This could potentially lead to compatibility issues with different R versions.

#!/bin/bash
# Description: Check r-base version constraints in similar packages
# Look for patterns of r-base version constraints in other recipes
fd -e yaml -e yml . recipes/ --exec rg -l 'r-base.*[0-9]'

recipes/r-archr/meta.yaml Show resolved Hide resolved
@jdblischak
Copy link
Member

The license is a mess. I was confused why I had it as GPL back in my original PR #36855. Looks like the file LICENSE and the field License in DESCRIPTION have never matched. From the first commit, GreenleafLab/ArchR@38b024e, LICENSE had MIT text and DESCRIPTION said License: GPL (>= 2), and 1.0.3 is the first release that has the fix from last year (GreenleafLab/ArchR@890223c, GreenleafLab/ArchR#1983)

@mfansler
Copy link
Member

FYI, I'm going to temp make the test passing optional so I can get an artifact to locally debug. Please no merges until I revert that. 🙏

@mfansler mfansler self-assigned this Nov 15, 2024
@mfansler mfansler marked this pull request as draft November 15, 2024 20:00
@mfansler
Copy link
Member

@BiocondaBot please fetch artifacts

@BiocondaBot
Copy link
Collaborator Author

Package(s) built are ready for inspection:

Arch Package Zip File / Repodata CI Instructions
linux-64 r-archr-1.0.3-r43hdbdd923_0.tar.bz2 linux-64.zip GitHub Actions
showYou may also use conda to install after downloading and extracting the zip file. conda install -c ./packages <package name>
osx-64 r-archr-1.0.3-r43h4a92bd6_0.tar.bz2 osx-64.zip GitHub Actions
showYou may also use conda to install after downloading and extracting the zip file. conda install -c ./packages <package name>

@mfansler
Copy link
Member

This fails in local tests as well. I've reported it at GreenleafLab/ArchR#2234

Looks like fragile assumptions about internal object structures. Probably need upper bound(s) on some (plotting?) dependencies.

Not sure I'll get back to this before Monday. Additional eyes welcome!

@mfansler
Copy link
Member

mfansler commented Nov 19, 2024

Looks like the biocontainer build section does not properly find the testthat files that are loaded through the test.source_files key. Is this a known bug?

Log Snippet (link)

12:41:03 BIOCONDA INFO (ERR) [Nov 19 12:41:03] SERR Error: `path` does not exist
12:41:03 BIOCONDA INFO (ERR) [Nov 19 12:41:03] SERR Execution halted
12:41:03 BIOCONDA INFO (ERR) [Nov 19 12:41:03] ERRO Task processing failed: Unexpected exit code [1] of container [fe3a4865cc0f step-cf14308070], container preserved
12:41:03 BIOCONDA INFO (OUT) ...
12:41:03 BIOCONDA ERROR COMMAND FAILED (exited with 1): mulled-build build-and-test r-archr=1.0.3--r43hdbdd923_0 -n biocontainers --test bash -c '/usr/local/env-execute true && . /usr/local/env-activate.sh && Rscript -e "library('"'"'ArchR'"'"')" && Rscript -e "testthat::test_file('"'"'tests/testthat.R'"'"', stop_on_failure=TRUE)"' --channels conda-forge,file:///opt/mambaforge/envs/bioconda/conda-bld,bioconda --involucro-path /opt/mambaforge/envs/bioconda/lib/python3.10/site-packages/bioconda_utils/involucro

12:41:03 BIOCONDA ERROR TEST FAILED: recipes/r-archr

@mfansler
Copy link
Member

In the meantime, I can work around this by simply installing the tests with the package. Then they can be referenced implicitly.

@mfansler
Copy link
Member

Seems that the container also cannot handle test.requires requirements. What to do 🤔 ? Can a test be skipped just for the container section?

this depends that `test.requires` are not installed for the container and checks lack of macs2 to skip
@mfansler mfansler marked this pull request as ready for review November 20, 2024 13:37
@mfansler mfansler added the please review & merge set to ask for merge label Nov 20, 2024
@mencian mencian merged commit 7f7efbe into master Nov 20, 2024
5 of 6 checks passed
@mencian mencian deleted the bump/r_archr branch November 20, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autobump Automatic Version Update new version please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants