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

Add r-pgenlibr #52020

Closed
wants to merge 4 commits into from
Closed

Add r-pgenlibr #52020

wants to merge 4 commits into from

Conversation

aryarm
Copy link
Contributor

@aryarm aryarm commented Nov 9, 2024

Adds the r-pgenlibr package from CRAN using https://github.com/bgruening/conda_r_skeleton_helper


Please read the guidelines for Bioconda recipes before opening a pull request (PR).

General instructions

  • If this PR adds or updates a recipe, use "Add" or "Update" appropriately as the first word in its title.
  • New recipes not directly relevant to the biological sciences need to be submitted to the conda-forge channel instead of Bioconda.
  • PRs require reviews prior to being merged. Once your PR is passing tests and ready to be merged, please issue the @BiocondaBot please add label command.
  • Please post questions on Gitter or ping @bioconda/core in a comment.

Instructions for avoiding API, ABI, and CLI breakage issues

Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify run_exports (see here for the rationale and comprehensive explanation).
Add a run_exports section like this:

build:
  run_exports:
    - ...

with ... being one of:

Case run_exports statement
semantic versioning {{ pin_subpackage("myrecipe", max_pin="x") }}
semantic versioning (0.x.x) {{ pin_subpackage("myrecipe", max_pin="x.x") }}
known breakage in minor versions {{ pin_subpackage("myrecipe", max_pin="x.x") }} (in such a case, please add a note that shortly mentions your evidence for that)
known breakage in patch versions {{ pin_subpackage("myrecipe", max_pin="x.x.x") }} (in such a case, please add a note that shortly mentions your evidence for that)
calendar versioning {{ pin_subpackage("myrecipe", max_pin=None) }}

while replacing "myrecipe" with either name if a name|lower variable is defined in your recipe or with the lowercase name of the package in quotes.

Bot commands for PR management

Please use the following BiocondaBot commands:

Everyone has access to the following BiocondaBot commands, which can be given in a comment:

@BiocondaBot please update Merge the master branch into a PR.
@BiocondaBot please add label Add the please review & merge label.
@BiocondaBot please fetch artifacts Post links to CI-built packages/containers.
You can use this to test packages locally.

Note that the @BiocondaBot please merge command is now depreciated. Please just squash and merge instead.

Also, the bot watches for comments from non-members that include @bioconda/<team> and will automatically re-post them to notify the addressed <team>.

Copy link
Contributor

coderabbitai bot commented Nov 9, 2024

📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request introduces two new files in the recipes/r-pgenlibr directory: build.sh and meta.yaml. The build.sh script is a Bash script that sets an environment variable to disable automatic brewing during installation and executes a command to install the package in build mode using R. The meta.yaml file defines the r-pgenlibr package with version 0.3.7, detailing its source URLs, SHA256 checksum, build configurations, and dependencies for both Windows and non-Windows environments. It includes a test section for verifying the installation by loading the library in R, along with metadata about the package such as its homepage, license, and maintainers. The file also contains commented-out sections for additional package details. Overall, these changes establish the necessary components for building and installing the r-pgenlibr package.

Suggested labels

please review & merge

Suggested reviewers

  • bgruening

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

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

53-57: Consider enhancing test coverage.

While the basic library load test is present, consider adding more comprehensive tests to verify package functionality.

Example of additional test commands:

 test:
   commands:
     - $R -e "library('pgenlibr')"           # [not win]
     - "\"%R%\" -e \"library('pgenlibr')\""  # [win]
+    - $R -e "stopifnot(packageVersion('pgenlibr') == '{{ version }}')"  # [not win]
+    - "\"%R%\" -e \"stopifnot(packageVersion('pgenlibr') == '{{ version }}')\""  # [win]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 44fba42 and c131391.

📒 Files selected for processing (3)
  • recipes/r-pgenlibr/bld.bat (1 hunks)
  • recipes/r-pgenlibr/build.sh (1 hunks)
  • recipes/r-pgenlibr/meta.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • recipes/r-pgenlibr/build.sh
🧰 Additional context used
🪛 yamllint
recipes/r-pgenlibr/meta.yaml

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

(syntax)

🔇 Additional comments (6)
recipes/r-pgenlibr/bld.bat (2)

1-1: LGTM: Correct R package installation command.

The command correctly uses environment variables %R% and %R_ARGS% to ensure flexible and configurable package installation.


2-2: LGTM: Essential error handling is in place.

The error handling ensures build failures are properly propagated to the CI system, which is crucial for maintaining package quality in Bioconda.

recipes/r-pgenlibr/meta.yaml (4)

1-14: LGTM! Package metadata and source configuration are well-structured.

The package version, source URLs, and checksum are properly defined following conda-forge R packaging conventions.

🧰 Tools
🪛 yamllint

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

(syntax)


15-25: LGTM! Build configuration is appropriate.

The build configuration includes necessary settings for both Windows and non-Windows environments, with proper rpaths and DSO whitelist configuration.


26-52: LGTM! Dependencies are well-defined with proper version pinning.

The requirements section correctly implements:

  • Version pinning through run_exports as requested in PR objectives
  • Platform-specific compiler requirements
  • Necessary R dependencies with version constraints

58-90: LGTM! Package information is comprehensive and well-documented.

The about section properly includes all necessary metadata, license information, and maintainer details.

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

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

37-40: Consider enhancing test coverage

The current test only verifies library loading. Consider adding basic functionality tests, such as:

  • Reading a small example .pgen file
  • Checking key function availability

Example addition:

 test:
   commands:
     - $R -e "library('pgenlibr')"           # [not win]
+    - $R -e "stopifnot('read_pgen' %in% getNamespaceExports('pgenlibr'))"  # [not win]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c131391 and a4150fa.

📒 Files selected for processing (1)
  • recipes/r-pgenlibr/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/r-pgenlibr/meta.yaml

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

(syntax)

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

13-19: Excellent use of run_exports for ABI stability!

The inclusion of run_exports with proper version pinning helps prevent ABI breakage in downstream packages, which aligns perfectly with the PR objectives.


44-47: Verify bioinformatics relevance

The package appears to be related to PLINK 2, which is a bioinformatics tool for genome association analysis. However, let's verify its usage in bioinformatics research.

#!/bin/bash
# Check if this package is used by other bioinformatics tools
rg -l "r-pgenlibr" recipes/*/meta.yaml

# Check the cited paper for bioinformatics relevance
echo "Verifying DOI: 10.1186/s13742-015-0047-8"

21-35: Consider adding R-base version constraint

While the dependencies are well-structured, consider adding a minimum R-base version constraint to ensure compatibility. This can be determined from the package's CRAN requirements.

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

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

37-40: Consider enhancing test coverage.

While loading the library is a basic test, consider adding more comprehensive tests:

  • Version check
  • Basic functionality test
 test:
   commands:
     - $R -e "library('pgenlibr')"           # [not win]
+    - $R -e "packageVersion('pgenlibr')"    # [not win]
+    - $R -e "stopifnot(packageVersion('pgenlibr') == '{{ version }}')"  # [not win]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a4150fa and 4cb2d2a.

📒 Files selected for processing (1)
  • recipes/r-pgenlibr/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/r-pgenlibr/meta.yaml

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

(syntax)

🔇 Additional comments (6)
recipes/r-pgenlibr/meta.yaml (6)

1-5: LGTM! Package naming and versioning follow conda conventions.

🧰 Tools
🪛 yamllint

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

(syntax)


7-11: LGTM! Source configuration includes proper fallback and verification.


41-50: LGTM! Package metadata is complete and well-structured.


52-54: LGTM! Maintainer section is correctly formatted.


21-35: Verify package's relevance to Bioconda.

The package appears to be a wrapper for PLINK 2's libraries, which is relevant to bioinformatics. However, please ensure this package isn't more appropriate for conda-forge as mentioned in the PR guidelines.

✅ Verification successful

Let me try a more focused search to verify if PLINK is relevant for bioinformatics.


Based on the search results, I can now provide a final response:

PLINK and its dependencies are appropriate for Bioconda

PLINK and PLINK2 are whole genome association analysis toolsets designed for bioinformatics applications, specifically for performing large-scale genomic analyses. Their presence in Bioconda is appropriate as they are core bioinformatics tools used in genomic research and analysis.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are related PLINK packages in bioconda
rg -l 'PLINK' recipes/**/meta.yaml

Length of output: 321781


Script:

#!/bin/bash
# Check PLINK's description and dependencies in its recipe
rg -A 10 "about:" recipes/plink/meta.yaml recipes/plink2/meta.yaml

Length of output: 882


13-19: Verify the pinning strategy in run_exports.

The x.x pin specification will allow updates to patch version but prevent minor version updates. Given this is a new package, please verify if this is the desired constraint level for downstream dependencies.

✅ Verification successful

Let me try a more focused search to verify the pinning strategy.


Let me try another search with a different approach:


Let me try one more search to look for similar R packages:


Based on the search results from multiple attempts, we can see that some R packages like r-corncob and r-archr use the same x.x pinning strategy in their run_exports section. This is a common pattern for R packages in the bioconda ecosystem. Additionally, the pinning strategy aligns with the goal of preventing ABI breakage while allowing patch updates.

The x.x pinning strategy in run_exports is appropriate.

The use of x.x max_pin in run_exports is a standard practice for R packages in bioconda, allowing updates to patch version while preventing minor version updates that could introduce ABI incompatibilities. This conservative approach helps maintain stability for downstream dependencies.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if similar R packages use the same pinning strategy
rg -l 'run_exports:.*pin_subpackage.*x.x' recipes/r-**/meta.yaml

Length of output: 321781


Script:

#!/bin/bash
# Search for run_exports sections with pin_subpackage in R packages
rg -A 3 'run_exports:.*\n.*pin_subpackage' recipes/r-*/meta.yaml

Length of output: 8210


Script:

#!/bin/bash
# Search for run_exports sections with pin_subpackage in R packages, using multiline mode
rg -U "run_exports:\s+- \{\{ pin_subpackage\('.*', max_pin='x.x'\) \}\}" recipes/r-*/meta.yaml

Length of output: 8306


Script:

#!/bin/bash
# Look for similar R packages that might have run_exports sections
rg -U "run_exports:.*pin_subpackage.*" recipes/r-pgenlibr/meta.yaml recipes/r-*/meta.yaml

Length of output: 8043

@aryarm
Copy link
Contributor Author

aryarm commented Nov 9, 2024

@BiocondaBot please update

@BiocondaBot
Copy link
Collaborator

I encountered an error updating your PR branch. You can report this to bioconda/core if you'd like.
-The Bot

@aryarm
Copy link
Contributor Author

aryarm commented Nov 9, 2024

@BiocondaBot please add label

@BiocondaBot BiocondaBot added the please review & merge set to ask for merge label Nov 9, 2024
@aryarm
Copy link
Contributor Author

aryarm commented Nov 11, 2024

My bad.

All CRAN packages that do not depend on a package in bioconda should be added to conda-forge instead. This is still the case if the CRAN package is directly related to bioinformatics.

I missed this in the bioconda contributing docs! I'll be closing this for now, then

@aryarm aryarm closed this Nov 11, 2024
@aryarm aryarm deleted the r-pgenlibr branch November 11, 2024 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants