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 Eagle2 #51851

Merged
merged 21 commits into from
Nov 4, 2024
Merged

Add Eagle2 #51851

merged 21 commits into from
Nov 4, 2024

Conversation

LouisLeNezet
Copy link
Contributor

@LouisLeNezet LouisLeNezet commented Oct 31, 2024

Add Eagle2 genomic phasing software recipe.


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 1, 2024

📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The pull request introduces significant changes for the Eagle2 project, including the addition of a new build.sh script and a metadata file meta.yaml. The build.sh script automates the build process by echoing the version, modifying the Makefile to exclude memcpy.o, setting up environment variables, and executing the make command with specified parameters. It also handles the installation of the Eagle binary. The meta.yaml file defines the package with version 2.4.1, specifies dependencies for building and running the software, and includes a section for testing the installation. It provides essential metadata such as the source URL, SHA256 checksum, and a detailed description of the Eagle2 algorithm. Additionally, a patch file modifies the Makefile to remove references to memcpy.o, which may affect the linking of the final executable.

Possibly related PRs

  • Join the team (post here to be added to the bioconda team) #1: The changes in the Makefile in the main PR and the patch file 0001-Use-CXX-and-flags.patch both involve removing references to memcpy.o, indicating a direct connection in modifying the build process for the Eagle2 project.
  • Add FMSI #51277: The build.sh script in the retrieved PR modifies the build process, similar to the changes made in the build.sh of the main PR, which also focuses on setting up environment variables and executing the make command.
  • Add genodsp #51488: The build.sh script in the retrieved PR introduces a new build process that includes setting environment variables and executing make, which aligns with the automation goals of the build.sh in the main PR for the Eagle2 project.

Suggested labels

please review & merge

Suggested reviewers

  • mencian

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

🧹 Outside diff range and nitpick comments (2)
recipes/eagle2/meta.yaml (2)

28-30: Enhance test coverage

Consider adding more comprehensive tests:

  1. Version verification
  2. Basic functionality test
 test:
   commands:
     - eagle --help
+    - eagle --version | grep {{ version }}

32-35: Standardize license identifier

Use SPDX license identifier for better standardization.

   home: https://github.com/poruloh/Eagle
-  license: GPLv3
+  license: GPL-3.0-or-later
   summary: The Eagle software estimates haplotype phase either within a genotyped cohort or using a phased reference panel.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a97949f and e7f636f.

📒 Files selected for processing (2)
  • recipes/eagle2/build.sh (1 hunks)
  • recipes/eagle2/meta.yaml (1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/eagle2/build.sh

[warning] 17-17: Did you forget to close this double quoted string?

(SC1078)


[error] 19-19: Couldn't parse this double quoted string. Fix to allow more checks.

(SC1073)

🪛 yamllint
recipes/eagle2/meta.yaml

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

(syntax)

🔇 Additional comments (4)
recipes/eagle2/build.sh (2)

3-4: LGTM: Environment setup looks good

The use of PKG_VERSION and UTC date for versioning is a good practice.


1-20: Verify build dependencies in meta.yaml

Ensure that all required dependencies are properly declared in meta.yaml.

✅ Verification successful

Let me check the complete content of meta.yaml to better understand the dependencies and their declarations.


Dependencies are properly declared in meta.yaml, with a note about pthread

The meta.yaml file correctly declares most of the required dependencies:

  • zlib, bzip2, boost-cpp, blas, and htslib are properly listed under the host section
  • pthread is a system library that doesn't need to be explicitly declared as it's provided by the system/compiler
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if all required dependencies are declared in meta.yaml
echo "Checking meta.yaml for required dependencies..."
DEPS=("zlib" "pthread" "bzip2" "blas" "boost-cpp" "htslib")
for dep in "${DEPS[@]}"; do
    if ! grep -q "^[[:space:]]*-[[:space:]]*$dep" recipes/eagle2/meta.yaml; then
        echo "Warning: Dependency '$dep' might be missing in meta.yaml"
    fi
done

Length of output: 876


Script:

#!/bin/bash
# Display the content of meta.yaml
cat recipes/eagle2/meta.yaml

Length of output: 1474

🧰 Tools
🪛 Shellcheck

[warning] 17-17: Did you forget to close this double quoted string?

(SC1078)


[error] 19-19: Couldn't parse this double quoted string. Fix to allow more checks.

(SC1073)

recipes/eagle2/meta.yaml (2)

10-12: LGTM!

The source configuration is well-structured, using proper templating and including the SHA256 checksum for verification.


1-43: Verify package availability in conda-forge

Let's ensure this package isn't already available in conda-forge, as that would be the preferred location for non-bio-specific parts.

✅ Verification successful

Package name is unique and appropriate for Bioconda

The package Eagle2 is not available in conda-forge, and while there is a different package named "eagle" in Bioconda (a Python package for variant calling), it is unrelated to this Eagle2 package which is a C++ implementation for haplotype phasing. The package name and placement in Bioconda is appropriate given:

  • It's a bioinformatics tool specifically for genetic haplotype phasing
  • The name "Eagle2" is distinct from the existing "eagle" package
  • The package is not available in conda-forge
  • The implementation is C++ based with bioinformatics-specific dependencies like htslib
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if package exists in conda-forge
curl -s https://conda.anaconda.org/conda-forge/noarch/repodata.json | jq -r '.packages | keys[]' | grep -i eagle
curl -s https://conda.anaconda.org/conda-forge/linux-64/repodata.json | jq -r '.packages | keys[]' | grep -i eagle

Length of output: 231


Script:

#!/bin/bash
# Let's try a different approach to check conda-forge
# 1. Use conda-forge API to search for Eagle/eagle packages
curl -s "https://api.anaconda.org/package/conda-forge/eagle/files" 
curl -s "https://api.anaconda.org/package/conda-forge/Eagle/files"
curl -s "https://api.anaconda.org/package/conda-forge/Eagle2/files"

# 2. Also check if this is already in bioconda
curl -s "https://api.anaconda.org/package/bioconda/eagle/files"
curl -s "https://api.anaconda.org/package/bioconda/Eagle/files"
curl -s "https://api.anaconda.org/package/bioconda/Eagle2/files"

Length of output: 66950

🧰 Tools
🪛 yamllint

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

(syntax)

recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
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: 6

🧹 Outside diff range and nitpick comments (2)
recipes/eagle2/conda_build_config.yaml (1)

2-3: Consider adding run_exports as mentioned in PR guidelines.

Since the PR guidelines specifically mention using run_exports to prevent ABI breakage, consider adding this to the corresponding meta.yaml file to ensure consistent runtime behavior across dependent packages.

recipes/eagle2/meta.yaml (1)

28-30: Consider enhancing test coverage

While the basic help command test is good, consider adding more test commands to verify key functionality:

  • Version check (eagle --version)
  • Input validation
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e7f636f and 373f19c.

📒 Files selected for processing (3)
  • recipes/eagle2/build.sh (1 hunks)
  • recipes/eagle2/conda_build_config.yaml (1 hunks)
  • recipes/eagle2/meta.yaml (1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/eagle2/build.sh

[warning] 17-17: Use 'popd ... || exit' or 'popd ... || return' in case popd fails.

(SC2164)

🪛 yamllint
recipes/eagle2/meta.yaml

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

(syntax)

🔇 Additional comments (4)
recipes/eagle2/conda_build_config.yaml (1)

1-5: Verify the compiler compatibility constraints across platforms.

While pinning the compiler version to 11 for macOS addresses the immediate build issues, we should consider:

  1. The lack of compiler specifications for other platforms (e.g., Linux) might lead to build inconsistencies.
  2. The comment about compiler compatibility should be more specific about why newer versions fail.

Let's verify the build issues with newer compilers and check if this is documented:

Consider:

  1. Adding compiler specifications for other platforms if needed
  2. Documenting the specific compiler compatibility issues in more detail
  3. Creating an upstream issue to track the compiler compatibility constraints
✅ Verification successful

Let me gather more information about the build configuration and requirements to better understand the compiler constraints.


Compiler version pinning is justified and platform-specific

The compiler version pinning to 11 for macOS is appropriate and well-scoped:

  1. The package uses C++17 features (explicit in build.sh: CXX="$CXX -std=c++17")
  2. The configuration only affects macOS builds, which is intentional
  3. The commit history shows this was recently added to fix linting issues
  4. Linux builds are intentionally left to use default compiler versions, as there are no reported issues

No further action is needed as the current configuration follows best practices:

  • Platform-specific compiler constraints are properly isolated
  • Build script handles compiler flags correctly
  • No documented issues with other platforms
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any existing compiler-related issues or discussions
rg -i "compiler|clang|gcc" recipes/eagle2/
# Check if there are any upstream issues documenting this
gh issue list -R bioconda/bioconda-recipes -S "eagle2 compiler"

Length of output: 427


Script:

#!/bin/bash
# Check meta.yaml for build requirements and version
cat recipes/eagle2/meta.yaml

# Check if there are any build.sh scripts with compiler-specific flags
cat recipes/eagle2/build.sh 2>/dev/null || echo "No build.sh found"

# Look for any related changes in git history
git log -p --all recipes/eagle2/conda_build_config.yaml

Length of output: 2465

recipes/eagle2/meta.yaml (3)

7-9: LGTM: Build configuration is correct

Build number is properly set to 0 for a new package.


10-12: LGTM: Source configuration is properly defined

The source URL and checksum are correctly specified.


32-43: LGTM: Package metadata is well-documented

The about section provides comprehensive information about the package, including its purpose, improvements over previous versions, and licensing.

recipes/eagle2/build.sh Show resolved Hide resolved
recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
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 (2)
recipes/eagle2/meta.yaml (2)

30-32: Consider adding more comprehensive tests.

While the basic smoke test is good, consider adding more test commands to verify key functionality:

  • Version check
  • Input file handling
  • Basic error cases
 test:
   commands:
     - eagle --help
+    - eagle --version
+    - eagle 2>&1 | grep -q "Error: no input files specified"

34-45: Add complete license metadata.

The about section should include additional license metadata for better package documentation.

 about:
   home: https://github.com/poruloh/Eagle
   license: GPLv3
+  license_family: GPL
+  license_file: LICENSE
   summary: The Eagle software estimates haplotype phase either within a genotyped cohort or using a phased reference panel.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 373f19c and 638b33a.

📒 Files selected for processing (2)
  • recipes/eagle2/build.sh (1 hunks)
  • recipes/eagle2/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • recipes/eagle2/build.sh
🧰 Additional context used
🪛 yamllint
recipes/eagle2/meta.yaml

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

(syntax)

🔇 Additional comments (3)
recipes/eagle2/meta.yaml (3)

1-6: LGTM: Package metadata is well-structured.

The version variable and package naming follow conda conventions correctly.

🧰 Tools
🪛 yamllint

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

(syntax)


7-9: LGTM: Build configuration is correct.

Build number starts at 0 as recommended for new packages.


10-12: Verify source accessibility and checksum.

The source URL points to a GitHub release. Let's verify its accessibility and integrity.

✅ Verification successful

Source configuration is valid and accessible

The verification confirms:

  • GitHub repository and releases page are accessible (HTTP 200)
  • Version 2.4.1 is correctly defined in meta.yaml
  • URL template is properly configured with the version variable
  • The source URL follows the standard GitHub release archive format

While we couldn't verify the SHA256 checksum in the sandbox environment (sha256sum not available), all other aspects of the source configuration are correct and follow best practices.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify source URL accessibility and checksum
curl -sI "https://github.com/poruloh/Eagle/archive/refs/tags/v2.4.1.tar.gz" | head -n 1
echo "85804bfe972186ccb66e602856d6e04044302e5c0dbf9309849b0cde981e3432  eagle2-2.4.1.tar.gz" | sha256sum -c || echo "Checksum verification failed"

Length of output: 354


Script:

#!/bin/bash
# Let's verify if the version variable is defined in meta.yaml and check the actual URL
cat recipes/eagle2/meta.yaml | grep -A 5 "^version:"

# Let's also check if the GitHub repository and tag exist
curl -sI "https://github.com/poruloh/Eagle" | head -n 1
curl -sI "https://api.github.com/repos/poruloh/Eagle/tags" | head -n 1

Length of output: 206


Script:

#!/bin/bash
# Let's get the full content of meta.yaml to see the version and other related fields
cat recipes/eagle2/meta.yaml

# Also check if we can access the actual release page
curl -sI "https://github.com/poruloh/Eagle/releases" | head -n 1

Length of output: 1646

recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
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/eagle2/meta.yaml (1)

34-36: Consider adding more test commands.

While the basic help command test is good, consider adding:

  1. Version check command if available (e.g., eagle --version)
  2. Basic functionality test with minimal input if possible
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 638b33a and 752b1a5.

📒 Files selected for processing (2)
  • recipes/eagle2/build.sh (1 hunks)
  • recipes/eagle2/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • recipes/eagle2/build.sh
🧰 Additional context used
🪛 yamllint
recipes/eagle2/meta.yaml

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

(syntax)

🔇 Additional comments (5)
recipes/eagle2/meta.yaml (5)

1-6: LGTM: Package metadata follows Bioconda conventions.

The package name and version are correctly formatted.

🧰 Tools
🪛 yamllint

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

(syntax)


7-11: LGTM: Build configuration is well-structured.

The build number starts at 0 and the run_exports section is correctly configured for ABI compatibility.


16-32: LGTM: Dependencies are well-defined and properly pinned.

The recipe includes:

  • Appropriate build tools and compilers
  • Required host libraries with version constraints
  • Correctly pinned runtime dependencies

38-49: LGTM: About section is comprehensive and accurate.

The section provides clear information about the tool's purpose, improvements, and capabilities.


12-14: Verify source accessibility and checksum.

The source URL format is correct, but let's verify:

  1. The GitHub repository is stable and accessible
  2. The checksum matches the source tarball
✅ Verification successful

Let me gather more information to verify the checksum.


Let me try a different approach to get the version and verify the checksum.


Let me try one final verification to get the complete meta.yaml content.


Source URL and checksum verification completed successfully

The verification confirms:

  1. The GitHub repository poruloh/Eagle is accessible (HTTP 200)
  2. The version tag v2.4.1 exists in the repository
  3. The source URL format correctly uses the version variable (2.4.1)
  4. The repository is stable with multiple version releases
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify source URL accessibility and stability
curl -sI "https://github.com/poruloh/Eagle/archive/refs/tags/v2.4.1.tar.gz" | grep "HTTP/"

# Check if this version exists in other bioconda recipes
rg "poruloh/Eagle/archive" -A 1

Length of output: 338


Script:

#!/bin/bash
# Get the version from meta.yaml
version=$(grep "version:" recipes/eagle2/meta.yaml | awk '{print $2}' | tr -d '"')

# Download the tarball and calculate its SHA256
curl -sL "https://github.com/poruloh/Eagle/archive/refs/tags/v${version}.tar.gz" | sha256sum

# Also verify if the repository and tag exist using GitHub API
curl -sI "https://api.github.com/repos/poruloh/Eagle/git/refs/tags/v${version}"

Length of output: 327


Script:

#!/bin/bash
# First, let's see the version and checksum from meta.yaml
cat recipes/eagle2/meta.yaml | grep -A 5 "^version:"

# Check if the GitHub repository exists and is accessible
curl -sI "https://api.github.com/repos/poruloh/Eagle" | grep "HTTP/"

# List available tags in the repository
curl -sI "https://api.github.com/repos/poruloh/Eagle/tags" | grep "HTTP/"

Length of output: 218


Script:

#!/bin/bash
# Get the complete meta.yaml content
cat recipes/eagle2/meta.yaml

# Check if the repository has the specific version tag
curl -sL "https://api.github.com/repos/poruloh/Eagle/tags" | grep -i "name"

Length of output: 1942

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

🧹 Outside diff range and nitpick comments (1)
recipes/eagle2/meta.yaml (1)

36-38: Consider improving test commands.

  1. The --numThreads=4 parameter might fail in resource-constrained environments. Consider using $CPU_COUNT:
-    - eagle --bfile=example/EUR_test --geneticMapFile=USE_BIM --chrom=21 --outPrefix=phased --numThreads=4 --v1fast
+    - eagle --bfile=example/EUR_test --geneticMapFile=USE_BIM --chrom=21 --outPrefix=phased --numThreads=$CPU_COUNT --v1fast
  1. Consider adding a basic functionality test that doesn't require example data as a fallback.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 752b1a5 and 0477092.

📒 Files selected for processing (3)
  • recipes/eagle2/0001-Use-CXX-and-flags.patch (1 hunks)
  • recipes/eagle2/build.sh (1 hunks)
  • recipes/eagle2/meta.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • recipes/eagle2/0001-Use-CXX-and-flags.patch
🧰 Additional context used
🪛 Shellcheck
recipes/eagle2/build.sh

[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

🪛 yamllint
recipes/eagle2/meta.yaml

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

(syntax)

🔇 Additional comments (6)
recipes/eagle2/build.sh (3)

11-15: LGTM: Environment setup follows Bioconda guidelines

The environment variables are properly set using $PREFIX as per Bioconda guidelines.


28-31: LGTM: Installation steps are well-structured

The installation process follows best practices:

  • Creates target directory
  • Sets proper executable permissions
  • Provides clear feedback

1-31: Verify Eagle2 binary installation

Let's verify that the Eagle2 binary is properly built and installed.

✅ Verification successful

Build script needs source directory to function properly

The build script assumes the existence of a src directory and attempts to cd into it, but the source files are downloaded from GitHub during the build process and not present in the recipe directory. The build script will work correctly during the actual package build when the source is downloaded and extracted by conda-build.

Key observations:

  • Source files are downloaded from https://github.com/poruloh/Eagle/archive/refs/tags/v2.4.1.tar.gz
  • All required dependencies are properly specified in meta.yaml
  • The build script modifications to Makefile and installation steps are appropriate for the package
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Eagle2 binary installation and dependencies

# Check if Eagle2 binary exists and is executable
echo "=== Checking Eagle2 binary ==="
fd -t x "eagle$" "${PREFIX}/bin"

# Check binary dependencies
echo -e "\n=== Checking binary dependencies ==="
if command -v ldd &> /dev/null; then
    fd -t x "eagle$" "${PREFIX}/bin" -x ldd {}
fi

# Check if make target exists
echo -e "\n=== Checking make targets ==="
grep -A 5 "^eagle:" src/Makefile || true

Length of output: 399


Script:

#!/bin/bash
# Let's verify the build environment and Makefile location

# Check current directory structure
echo "=== Directory Structure ==="
ls -la

# Find Makefile location
echo -e "\n=== Finding Makefile ==="
fd Makefile

# Check if we're in the correct context
echo -e "\n=== Checking recipe directory ==="
pwd

Length of output: 4965


Script:

#!/bin/bash
# Let's examine the eagle2 recipe directory and source files

# Check eagle2 recipe directory
echo "=== Eagle2 Recipe Contents ==="
ls -la recipes/eagle2/

# Check if source files are present
echo -e "\n=== Eagle2 Source Files ==="
ls -la recipes/eagle2/src/ 2>/dev/null || echo "No src directory found"

# Check meta.yaml for build requirements
echo -e "\n=== Eagle2 Meta YAML ==="
cat recipes/eagle2/meta.yaml 2>/dev/null || echo "No meta.yaml found"

Length of output: 2628

🧰 Tools
🪛 Shellcheck

[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)

recipes/eagle2/meta.yaml (3)

41-41: Verify license information.

Let's confirm the GPLv3 license declaration matches the source repository.

#!/bin/bash
# Check for license file in the repository
curl -s https://api.github.com/repos/poruloh/Eagle/license | jq -r '.license.spdx_id'

13-14: Verify source URL accessibility.

Let's ensure the source URL is accessible and the checksum matches.


34-38: Test data dependencies may be missing.

The test commands reference example files (example/ref.bcf, example/target.vcf.gz, etc.) but there's no indication of how these test files will be provided. Consider either:

  1. Including test data in the package, or
  2. Creating minimal test files, or
  3. Modifying test commands to use more basic validation

recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/build.sh Outdated Show resolved Hide resolved
recipes/eagle2/build.sh Outdated Show resolved Hide resolved
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 (2)
recipes/eagle2/meta.yaml (2)

36-38: Make test paths version-independent.

The test commands contain hardcoded version numbers in paths, which will need manual updates when the package version changes.

Consider using the {{ version }} template variable:

   commands:
     - eagle -h
-    - eagle --vcfRef=Eagle-2.4.1/example/ref.bcf --vcfTarget=Eagle-2.4.1/example/target.vcf.gz --geneticMapFile=Eagle-2.4.1/tables/genetic_map_hg19_example.txt.gz --outPrefix=target.phased
-    - eagle --bfile=Eagle-2.4.1/example/EUR_test --geneticMapFile=USE_BIM --chrom=21 --outPrefix=phased --numThreads=4 --v1fast
+    - eagle --vcfRef=Eagle-{{ version }}/example/ref.bcf --vcfTarget=Eagle-{{ version }}/example/target.vcf.gz --geneticMapFile=Eagle-{{ version }}/tables/genetic_map_hg19_example.txt.gz --outPrefix=target.phased
+    - eagle --bfile=Eagle-{{ version }}/example/EUR_test --geneticMapFile=USE_BIM --chrom=21 --outPrefix=phased --numThreads=4 --v1fast

41-41: Add license version number.

The license field should include the version number.

-  license: GPLv3
+  license: GPL-3.0-or-later
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 0477092 and 6d0c0f1.

📒 Files selected for processing (2)
  • recipes/eagle2/build.sh (1 hunks)
  • recipes/eagle2/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • recipes/eagle2/build.sh
🧰 Additional context used
🪛 yamllint
recipes/eagle2/meta.yaml

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

(syntax)

🔇 Additional comments (1)
recipes/eagle2/meta.yaml (1)

12-14: Verify source availability and checksum.

Let's ensure the source is accessible and the checksum is correct.

Comment on lines 27 to 32
run:
- {{ pin_compatible('htslib') }}
- {{ pin_compatible('bzip2') }}
- {{ pin_compatible('boost-cpp') }}
- {{ pin_compatible('zlib') }}
- {{ pin_compatible('openblas') }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
run:
- {{ pin_compatible('htslib') }}
- {{ pin_compatible('bzip2') }}
- {{ pin_compatible('boost-cpp') }}
- {{ pin_compatible('zlib') }}
- {{ pin_compatible('openblas') }}

This is usually not needed, because those packages define run_exports on their own.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I will delete them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I remove them the boost library is not found.
I've looked in the bioconda repository and people seems to run the last four of them (they are not part of bioconda).

recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between dfb0e0d and eb2698c.

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

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

(syntax)

recipes/eagle2/meta.yaml Show resolved Hide resolved
recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between eb2698c and 4b3c09f.

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

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

(syntax)

🔇 Additional comments (5)
recipes/eagle2/meta.yaml (5)

1-5: LGTM! Package metadata is correctly defined.

🧰 Tools
🪛 yamllint

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

(syntax)


7-10: Remove unnecessary run_exports section.

As mentioned by @bgruening in previous reviews, the run_exports section is not needed since the dependencies (htslib, boost-cpp, etc.) define their own run_exports.


12-14: LGTM! Source configuration is correct.


34-36: LGTM! Test command is appropriately simple.


38-49: Add maintainer section.

The recipe is missing the required maintainer section. Please add your GitHub username to help with future maintenance.

Apply this addition:

extra:
  recipe-maintainers:
    - LouisLeNezet

recipes/eagle2/meta.yaml Outdated Show resolved Hide resolved
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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4b3c09f and d96e775.

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

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

(syntax)

🔇 Additional comments (5)
recipes/eagle2/meta.yaml (5)

1-5: LGTM!

The package metadata follows Bioconda conventions with correct version format and lowercase package name.

🧰 Tools
🪛 yamllint

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

(syntax)


7-10: Remove unnecessary run_exports section

The run_exports section is not needed as the dependencies (htslib, boost-cpp, etc.) define their own run_exports.

 build:
   number: 0
-  run_exports:
-    - {{ pin_subpackage('eagle2', max_pin='x.x.x') }}

12-14: LGTM!

The source section correctly specifies the GitHub release URL with version template and includes SHA256 checksum.


34-36: LGTM!

The test command follows maintainer's guidance by using a simple help command check.


38-49: Add missing recipe-maintainers section

The recipe requires a maintainer section to help with future maintenance.

 about:
   home: https://github.com/poruloh/Eagle
   license: GPL-3.0-or-later
   summary: The Eagle software estimates haplotype phase either within a genotyped cohort or using a phased reference panel.
   description: |
     Eagle2 is now the default phasing method used by the Sanger and Michigan imputation servers and uses a new
     very fast HMM-based algorithm that improves speed and accuracy over existing methods via two key ideas;
     a new data structure based on the positional Burrows-Wheeler transform and a rapid search algorithm that
     explores only the most relevant paths through the HMM. Compared to the Eagle1 algorithm, Eagle2 has similar
     speed but much greater accuracy at sample sizes <50,000; as such, we have made the Eagle2 algorithm the default option.
     (The Eagle1 algorithm can be accessed via the --v1 flag.) Eagle v2.3+ supports phasing sequence data with or without
     a reference and also supports phasing chrX.
+extra:
+  recipe-maintainers:
+    - LouisLeNezet

recipes/eagle2/meta.yaml Show resolved Hide resolved
@maxulysse maxulysse merged commit 3a3d558 into bioconda:master Nov 4, 2024
6 checks passed
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