Update atlas to version 2.0.0-rc.5 #51996
Conversation
📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant changes to the Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
recipes/atlas/build.sh (1)
5-5: Add build type configuration and fix formatting.The CMake configuration should specify the build type and the line has extra spacing.
- cmake .. -GNinja -DCONDA=ON -DCMAKE_CXX_COMPILER=${CXX} -DCMAKE_LIBRARY_PATH=${CONDA_PREFIX}/lib -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/include +cmake .. -GNinja \ + -DCONDA=ON \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_CXX_COMPILER=${CXX} \ + -DCMAKE_LIBRARY_PATH=${CONDA_PREFIX}/lib \ + -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/includerecipes/atlas/meta.yaml (2)
29-29: Fix trailing whitespace after 'openmp'Remove the trailing whitespace after 'openmp' in both host and run requirements.
Also applies to: 38-38
🧰 Tools
🪛 yamllint
[error] 29-29: trailing spaces
(trailing-spaces)
42-48: Consider improving test robustnessThe test commands form a chain where each step depends on files generated by previous steps. Consider:
- Adding error checking between steps
- Cleaning up temporary files after tests
- Adding timeouts for long-running operations
Example improvement:
commands: - atlas | grep "successfully" - atlas simulate + - test -f ATLAS_simulations.bam || exit 1 - atlas HKY85 --bam ATLAS_simulations.bam --fasta ATLAS_simulations.fasta + - test -f ATLAS_simulations.glf.gz || exit 1 - atlas GLF --bam ATLAS_simulations.bam - atlas saf --glf ATLAS_simulations.glf.gz --fasta ATLAS_simulations.fasta - atlas majorMinor --glf ATLAS_simulations.glf.gz + - rm -f ATLAS_simulations.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/atlas/build.sh(1 hunks)recipes/atlas/meta.yaml(1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/atlas/build.sh
[warning] 4-4: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🪛 yamllint
recipes/atlas/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/atlas/build.sh (1)
1-9: Verify downstream package compatibility.
As this is a major version update (2.0.0-rc.5) with a complete build system change, we should verify potential impacts on downstream packages.
🧰 Tools
🪛 Shellcheck
[warning] 4-4: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
recipes/atlas/meta.yaml (2)
12-14: LGTM! Good practice for ABI compatibility
The run_exports configuration with x-pin will help prevent ABI breakage in downstream packages, which is crucial for a major version update.
17-39: Verify OpenMP compatibility across platforms
The addition of OpenMP as a dependency needs verification for cross-platform compatibility, especially for the newly added platforms (linux-aarch64, osx-arm64).
✅ Verification successful
Let me try a different approach to verify OpenMP compatibility on these platforms.
Based on the search results, I can now provide a definitive response about OpenMP compatibility across platforms.
OpenMP is well-supported across bioconda packages for the target platforms
The search results show extensive OpenMP usage across bioconda packages with proper platform-specific handling:
- For macOS (osx-arm64), packages use
llvm-openmpwith the conditional selector# [osx] - For Linux (linux-aarch64), packages use the standard
openmppackage with the conditional selector# [linux]
Many bioconda packages (like mmseqs2, spades, bowtie2, etc.) successfully use this pattern to support OpenMP across both platforms. The recipe's approach of using openmp as a dependency aligns with established practices in the ecosystem.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other bioconda packages using OpenMP support these platforms
rg -l "openmp" | grep meta.yaml | xargs grep -l "osx-arm64\|linux-aarch64"
Length of output: 2017
Script:
#!/bin/bash
# Check if the recipe explicitly skips these platforms
cat recipes/atlas/meta.yaml | grep -E "skip|skip_platforms"
# Check build.sh for any platform-specific OpenMP configurations
cat recipes/atlas/build.sh | grep -i "openmp"
# Look for other recipes using OpenMP in their build process
rg -l "openmp" --type yaml | while read -r file; do
echo "=== $file ==="
grep -i "skip\|platform\|openmp" "$file" || true
echo
done
Length of output: 138076
🧰 Tools
🪛 yamllint
[error] 29-29: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
recipes/atlas/build.sh
Outdated
| mkdir -p build | ||
| cd build |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for directory operations.
The cd command should handle potential failures to prevent silent errors in the build process.
mkdir -p build
-cd build
+cd build || exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mkdir -p build | |
| cd build | |
| mkdir -p build | |
| cd build || exit 1 |
🧰 Tools
🪛 Shellcheck
[warning] 4-4: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
recipes/atlas/build.sh
Outdated
| mkdir -p ${PREFIX}/bin | ||
| cp atlas ${PREFIX}/bin |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve binary installation robustness.
The installation steps should handle errors and ensure proper executable permissions.
mkdir -p ${PREFIX}/bin
-cp atlas ${PREFIX}/bin
+cp atlas ${PREFIX}/bin/atlas || exit 1
+chmod +x ${PREFIX}/bin/atlas📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mkdir -p ${PREFIX}/bin | |
| cp atlas ${PREFIX}/bin | |
| mkdir -p ${PREFIX}/bin | |
| cp atlas ${PREFIX}/bin/atlas || exit 1 | |
| chmod +x ${PREFIX}/bin/atlas |
recipes/atlas/build.sh
Outdated
| mkdir -p build | ||
| cd build | ||
| cmake .. -GNinja -DCONDA=ON -DCMAKE_CXX_COMPILER=${CXX} -DCMAKE_LIBRARY_PATH=${CONDA_PREFIX}/lib -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/include | ||
| ninja |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for build command.
The build command should exit on failure to prevent incomplete installations.
-ninja
+ninja || exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ninja | |
| ninja || exit 1 |
recipes/atlas/meta.yaml
Outdated
| {% set version = "2.0.0-rc.5" %} | ||
|
|
||
| package: | ||
| name: atlas | ||
| version: {{ version }} | ||
| version: 2.0.0 | ||
|
|
||
| source: | ||
| url: https://bitbucket.org/wegmannlab/atlas/get/{{ version }}.tar.gz | ||
| sha256: 7b65eae5ab7ff439accd53febf8caa15ad9d8230278eee31fd5c2e95bf021990 | ||
| sha256: fabbde9a4af8053f748568c666fbf0210e318f7794bd8d8aed827d5da37314d5 |
There was a problem hiding this comment.
Version inconsistency detected between template variable and package version
The template sets version to "2.0.0-rc.5" but the package version is set to "2.0.0". This inconsistency could cause issues with dependency resolution and version tracking.
Apply this diff to maintain consistency:
{% set version = "2.0.0-rc.5" %}
package:
name: atlas
- version: 2.0.0
+ version: {{ version }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {% set version = "2.0.0-rc.5" %} | |
| package: | |
| name: atlas | |
| version: {{ version }} | |
| version: 2.0.0 | |
| source: | |
| url: https://bitbucket.org/wegmannlab/atlas/get/{{ version }}.tar.gz | |
| sha256: 7b65eae5ab7ff439accd53febf8caa15ad9d8230278eee31fd5c2e95bf021990 | |
| sha256: fabbde9a4af8053f748568c666fbf0210e318f7794bd8d8aed827d5da37314d5 | |
| {% set version = "2.0.0-rc.5" %} | |
| package: | |
| name: atlas | |
| version: {{ version }} | |
| source: | |
| url: https://bitbucket.org/wegmannlab/atlas/get/{{ version }}.tar.gz | |
| sha256: fabbde9a4af8053f748568c666fbf0210e318f7794bd8d8aed827d5da37314d5 |
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/atlas/meta.yaml
Outdated
| extra: | ||
| additional-platforms: | ||
| - linux-aarch64 | ||
| - osx-arm64 |
There was a problem hiding this comment.
💡 Codebase verification
Dependencies not fully available for ARM platforms
Based on the verification results, there are compatibility concerns for ARM platforms:
fmtandlapackare available for both linux-aarch64 and osx-arm64armadillois missing for both ARM platformshtslibis missing for both ARM platforms
The package cannot be built for ARM platforms without all required dependencies being available. Please either:
- Remove the ARM platform support until all dependencies are available
- Work with conda-forge to get the missing dependencies built for ARM platforms
🔗 Analysis chain
Verify build infrastructure for new platforms
The addition of arm64 platforms is significant. Please verify:
- Build infrastructure supports these platforms
- All dependencies are available for arm64
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if all dependencies are available for arm64 platforms
echo "Checking host dependencies..."
for dep in armadillo fmt htslib lapack nlohmann_json openblas openmp zlib; do
echo "Checking $dep..."
rg "name:\s*$dep" | xargs grep -l "osx-arm64\|linux-aarch64"
done
Length of output: 2017
Script:
#!/bin/bash
# Let's try a different approach to check dependencies
# First, let's find the requirements section in the meta.yaml
echo "Checking requirements in meta.yaml..."
rg -A 10 "requirements:" recipes/atlas/meta.yaml
# Then let's check if these dependencies are available in conda-forge
echo -e "\nChecking conda-forge packages..."
gh api -H "Accept: application/vnd.github+json" \
/repos/conda-forge/feedstock-outputs/contents \
| jq -r '.[] | select(.name | test("(armadillo|fmt|htslib|lapack|nlohmann_json|openblas|openmp|zlib).*json$")) | .name'
Length of output: 575
Script:
#!/bin/bash
# Let's check the conda-forge repository directly for arm64 support in these packages
echo "Checking armadillo..."
curl -s "https://conda.anaconda.org/conda-forge/linux-aarch64/repodata.json" | jq -r '.packages | keys[] | select(startswith("armadillo-"))'
echo -e "\nChecking fmt..."
curl -s "https://conda.anaconda.org/conda-forge/linux-aarch64/repodata.json" | jq -r '.packages | keys[] | select(startswith("fmt-"))'
echo -e "\nChecking htslib..."
curl -s "https://conda.anaconda.org/conda-forge/linux-aarch64/repodata.json" | jq -r '.packages | keys[] | select(startswith("htslib-"))'
echo -e "\nChecking lapack..."
curl -s "https://conda.anaconda.org/conda-forge/linux-aarch64/repodata.json" | jq -r '.packages | keys[] | select(startswith("lapack-"))'
# Also check for osx-arm64
echo -e "\nChecking osx-arm64 packages..."
curl -s "https://conda.anaconda.org/conda-forge/osx-arm64/repodata.json" | jq -r '.packages | keys[] | select(startswith("armadillo-") or startswith("fmt-") or startswith("htslib-") or startswith("lapack-"))'
Length of output: 1795
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/atlas/build.sh(1 hunks)recipes/atlas/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/atlas/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (7)
recipes/atlas/build.sh (2)
10-10: LGTM! Build command looks good.
The build command includes proper error handling.
12-14: LGTM! Installation steps look good.
The installation steps include proper error handling and file permissions.
recipes/atlas/meta.yaml (5)
12-14: Good addition of run_exports configuration!
The pin_subpackage directive will help prevent API/ABI breakage by ensuring proper version constraints in dependent packages.
8-9: Verify source URL and checksum.
Let's verify that the URL and checksum match the official release.
#!/bin/bash
# Verify the source URL and checksum
URL="https://bitbucket.org/wegmannlab/atlas/get/2.0.0-rc.5.tar.gz"
echo "Checking URL accessibility..."
curl -I "$URL"
# Note: We can't verify the checksum directly as we shouldn't download
# and execute files from the repository in the sandbox environment22-22: Verify sysroot version pinning impact.
The recipe pins sysroot_linux-64 to exactly 2.17. This strict pinning might cause issues with different build environments.
#!/bin/bash
# Check if other bioconda packages use similar sysroot pinning
echo "Checking sysroot pinning patterns in other recipes..."
rg "sysroot_linux-64\s*=\s*2\.17" recipes/Also applies to: 32-32, 43-43
57-58: Verify license change from GPLv3 to MPL-2.0.
The license has been changed from GPLv3 to MPL-2.0. This is a significant change that needs verification.
#!/bin/bash
# Check for license file and its content
echo "Checking for license file..."
rg -l "Mozilla Public License" --type-add 'license:*LICENSE*' -t license
# Check commit history for license changes
echo "Checking commit history for license changes..."
git log -p | rg -A 5 -B 5 "GPL.*MPL|Mozilla Public License"47-54: Verify test commands documentation.
The test commands look comprehensive. Let's verify if these commands are documented in the package's documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
recipes/atlas/meta.yaml (1)
47-54: Consider enhancing test error handlingWhile the test suite covers core functionality well, consider making it more robust by adding error handling to individual commands.
- - atlas | grep "successfully" - - atlas simulate - - atlas HKY85 --bam ATLAS_simulations.bam --fasta ATLAS_simulations.fasta - - atlas GLF --bam ATLAS_simulations.bam - - atlas saf --glf ATLAS_simulations.glf.gz --fasta ATLAS_simulations.fasta - - atlas majorMinor --glf ATLAS_simulations.glf.gz + - atlas | grep "successfully" || exit 1 + - atlas simulate || exit 1 + - atlas HKY85 --bam ATLAS_simulations.bam --fasta ATLAS_simulations.fasta || exit 1 + - atlas GLF --bam ATLAS_simulations.bam || exit 1 + - atlas saf --glf ATLAS_simulations.glf.gz --fasta ATLAS_simulations.fasta || exit 1 + - atlas majorMinor --glf ATLAS_simulations.glf.gz || exit 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/atlas/build.sh(1 hunks)recipes/atlas/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/atlas/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (5)
recipes/atlas/build.sh (1)
12-14: Consider using CMake's installation mechanism.
Manual installation bypasses CMake's built-in installation system. Using ninja install would be more reliable and could handle additional files defined in CMake installation targets.
-mkdir -p ${PREFIX}/bin
-cp atlas ${PREFIX}/bin || exit 1
-chmod +x ${PREFIX}/bin/atlas
+ninja install || exit 1This assumes proper installation targets are configured in CMakeLists.txt. Would you like me to verify the CMake configuration or help set up the installation targets?
recipes/atlas/meta.yaml (4)
12-14: LGTM! Good practice with run_exports
The build number reset and run_exports configuration are appropriate for this major version update. The pinning strategy will help maintain ABI compatibility.
17-44: Build system transition looks good
The transition from Make to CMake+Ninja is well structured with appropriate dependencies. The synchronized host and run requirements ensure consistent runtime behavior.
A few notes:
- Platform-specific OpenMP implementations are correctly handled
- The sysroot pinning helps maintain glibc compatibility
- The addition of fmt, htslib, and nlohmann_json suggests new features in 2.0.0
57-59: Verify license change documentation
The license has changed from GPLv3 to MPL-2.0. This is a significant change that should be documented.
#!/bin/bash
# Check for license change documentation
echo "Checking repository for license change documentation..."
rg -i "license.*(change|switch|mov.*to.*mpl)"
echo "Verifying current license file..."
find . -type f -iname "license*" -o -iname "copying*" | xargs cat8-9: Verify source integrity
Let's verify the SHA256 checksum matches the source tarball.
recipes/atlas/build.sh
Outdated
| cmake . -GNinja \ | ||
| -DCONDA=ON \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -DCMAKE_CXX_COMPILER=${CXX} \ | ||
| -DCMAKE_LIBRARY_PATH=${CONDA_PREFIX}/lib \ | ||
| -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/include |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add CMAKE_INSTALL_PREFIX to ensure correct installation paths.
The CMake configuration should explicitly set the installation prefix to ensure files are installed in the correct conda environment location.
cmake . -GNinja \
-DCONDA=ON \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_CXX_COMPILER=${CXX} \
-DCMAKE_LIBRARY_PATH=${CONDA_PREFIX}/lib \
- -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/include
+ -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/include \
+ -DCMAKE_INSTALL_PREFIX=${PREFIX}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cmake . -GNinja \ | |
| -DCONDA=ON \ | |
| -DCMAKE_BUILD_TYPE=Release \ | |
| -DCMAKE_CXX_COMPILER=${CXX} \ | |
| -DCMAKE_LIBRARY_PATH=${CONDA_PREFIX}/lib \ | |
| -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/include | |
| cmake . -GNinja \ | |
| -DCONDA=ON \ | |
| -DCMAKE_BUILD_TYPE=Release \ | |
| -DCMAKE_CXX_COMPILER=${CXX} \ | |
| -DCMAKE_LIBRARY_PATH=${CONDA_PREFIX}/lib \ | |
| -DCMAKE_INCLUDE_PATH=${CONDA_PREFIX}/include \ | |
| -DCMAKE_INSTALL_PREFIX=${PREFIX} |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/atlas/build.sh(1 hunks)recipes/atlas/meta.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- recipes/atlas/build.sh
🧰 Additional context used
🪛 yamllint
recipes/atlas/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (6)
recipes/atlas/meta.yaml (6)
1-5: Version mismatch between template and package version
This issue was previously identified in an existing review comment. The version inconsistency needs to be fixed as suggested.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
11-14: LGTM! Good practice for ABI compatibility
The run_exports configuration with pin_subpackage will help prevent ABI breakage in downstream packages, which is especially important for a major version update.
46-54: Great test coverage!
The test commands provide good coverage of the package's main functionality, including simulation, various analysis modes, and proper cleanup.
61-64: ARM platform compatibility concerns
The concerns about ARM platform support and dependency availability were previously raised in an existing review comment. Please address those concerns before enabling these platforms.
16-44: Dependencies look good, but verify compiler requirements
The build and runtime dependencies are well-structured with appropriate platform-specific conditionals. The transition to CMake build system is properly reflected in the requirements.
✅ Verification successful
Dependencies and compiler requirements are properly configured
The verification shows that:
- The C++ compiler requirement is correctly specified using conda-forge's compiler syntax
{{ compiler('cxx') }} - Build requirements properly include CMake, git, and ninja for the build system
- CMake configuration properly handles compiler detection and environment variables
- The codebase has appropriate compiler version checks and compatibility handling
The requirements section in meta.yaml is well-structured and includes all necessary dependencies for building and running the package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify compiler compatibility
echo "Checking compiler requirements..."
rg "{{ compiler\('cxx'\) }}" | grep -B2 -A2 "version"
# Verify if all dependencies are available in conda-forge
echo -e "\nChecking dependency availability..."
for dep in armadillo fmt htslib lapack nlohmann_json openblas; do
echo "Checking $dep..."
curl -s "https://api.anaconda.org/package/conda-forge/$dep" | jq -r '.versions[]' | sort -V | tail -n1
done
Length of output: 499
Script:
#!/bin/bash
# Let's verify dependencies and compiler requirements separately
# Check compiler requirements in meta.yaml
echo "Checking compiler requirements in meta.yaml..."
rg -A2 -B2 "compiler\('cxx'\)" recipes/atlas/meta.yaml
# Check build system requirements
echo -e "\nChecking build system requirements..."
rg -A5 "requirements:" recipes/atlas/meta.yaml
# Check if there are any specific compiler version constraints in CMake files
echo -e "\nChecking CMake compiler requirements..."
fd -e cmake -x cat {} \; | rg -i "cmake_minimum|compiler|cxx"
Length of output: 3129
7-9: Verify source URL and checksum
Let's verify the source URL resolves correctly and the checksum matches.
recipes/atlas/meta.yaml
Outdated
| about: | ||
| home: https://bitbucket.org/wegmannlab/atlas/wiki/Home | ||
| license: GPLv3 | ||
| summary: "ATLAS, a suite of methods to accurately genotype and estimate genetic diversity" | ||
| home: https://atlaswiki.netlify.app | ||
| license: MPL-2.0 | ||
| summary: "ATLAS: Analysis Tools for Ancient and Low-depth Samples" | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
License discrepancy needs attention
The source code contains a GPLv3 license (in COPYING file) but the recipe specifies MPL-2.0. This is a significant discrepancy that needs to be addressed. Additionally, while the documentation URL is accessible (HTTP 200), the license change from GPL to MPL requires verification from the project maintainers.
- The license in
meta.yamlshould be updated toGPL-3.0to match the actual license in the source code - Alternatively, if MPL-2.0 is the intended new license, the project maintainers need to update the source code's license file
🔗 Analysis chain
Verify new home URL and license
Let's confirm the new documentation URL is accessible and the license change is intentional.
Let me verify the license content since we found a COPYING file in the source.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify home URL
echo "Checking documentation URL..."
curl -sI "https://atlaswiki.netlify.app" | head -n 1
# Check license in source
echo -e "\nVerifying license..."
curl -sL "https://bitbucket.org/wegmannlab/atlas/get/2.0.0-rc.5.tar.gz" | tar -tz | grep -i "license\|copying"
Length of output: 499
Script:
#!/bin/bash
# Download and extract the license file
curl -sL "https://bitbucket.org/wegmannlab/atlas/get/2.0.0-rc.5.tar.gz" | tar -xz -O "wegmannlab-atlas-f1941eafc62f/COPYING"
Length of output: 35272
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/atlas/meta.yaml (1)
47-54: Consider enhancing test robustnessWhile the test coverage is comprehensive, consider these improvements:
- Add error checking between commands to fail fast
- Consider preserving test artifacts on failure for debugging
test: commands: - - atlas | grep "successfully" - - atlas simulate - - atlas HKY85 --bam ATLAS_simulations.bam --fasta ATLAS_simulations.fasta - - atlas GLF --bam ATLAS_simulations.bam - - atlas saf --glf ATLAS_simulations.glf.gz --fasta ATLAS_simulations.fasta - - atlas majorMinor --glf ATLAS_simulations.glf.gz - - rm ATLAS_* *.parameters saf.saf.* || exit 1 + - | + set -e + atlas | grep "successfully" + atlas simulate || { echo "Simulation failed"; exit 1; } + atlas HKY85 --bam ATLAS_simulations.bam --fasta ATLAS_simulations.fasta || { echo "HKY85 failed"; exit 1; } + atlas GLF --bam ATLAS_simulations.bam || { echo "GLF failed"; exit 1; } + atlas saf --glf ATLAS_simulations.glf.gz --fasta ATLAS_simulations.fasta || { echo "SAF failed"; exit 1; } + atlas majorMinor --glf ATLAS_simulations.glf.gz || { echo "majorMinor failed"; exit 1; } + # Only cleanup on success + rm ATLAS_* *.parameters saf.saf.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/atlas/build.sh(1 hunks)recipes/atlas/meta.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- recipes/atlas/build.sh
🧰 Additional context used
🪛 yamllint
recipes/atlas/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/atlas/meta.yaml (4)
1-9: Version inconsistency between template and source URL
The version inconsistency issue was previously identified and still needs to be addressed.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
12-14: LGTM: Appropriate build configuration
The build number reset and run_exports configuration are correctly implemented for the major version update.
17-44: ARM platform dependency compatibility issues
The ARM platform compatibility issues were previously identified and still need to be addressed.
57-59: License specification discrepancy
The license discrepancy issue was previously identified and still needs to be addressed.
* Update sylph to 0.7.0 * Update meta.yaml * Update build.sh --------- Co-authored-by: Joshua Zhuang <[email protected]>
* Update bioblend to 1.4.0 * add setuptools to host --------- Co-authored-by: mencian <[email protected]>
* Update gadma to 2.0.1 * clean up recipe * Update gadma to 2.0.2 * `trapz` has been removed in scipy 1.14.0 --------- Co-authored-by: mencian <[email protected]> Co-authored-by: Joshua Zhuang <[email protected]>
New version of atlas with lots of new features:
https://atlaswiki.netlify.app/
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add labelcommand.@bioconda/corein 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_exportssection like this:with
...being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}while replacing
"myrecipe"with eithernameif aname|lowervariable 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@BiocondaBot please add labelplease review & mergelabel.@BiocondaBot please fetch artifactsYou can use this to test packages locally.
Note that the
@BiocondaBot please mergecommand 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>.