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 meta.yaml #51670

Closed
wants to merge 12 commits into from
Closed

Update meta.yaml #51670

wants to merge 12 commits into from

Conversation

KennthShang
Copy link
Contributor

Describe your pull request here


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 Oct 25, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

The pull request introduces significant modifications to the recipes/phabox/meta.yaml file, primarily focusing on the package's configuration. The build section has been updated to increment the build number from 0 to 1. In the requirements section, the host dependencies have been revised to remove pip, while the run dependencies have undergone substantial changes: new dependencies diamond, blast, mcl, and triton have been added with specific version constraints. Existing dependencies have also been updated to include specific versions, such as datasets to >=3.0.2, pandas to >=2.2.3, pyarrow to >=16.1.0, pytorch to >=2.4.0, and scipy to >=1.14.1. The transformers dependency has been updated to >=4.11.3. The test section remains unchanged, still containing a command to check the help output of the phabox2 command-line interface. Additionally, the about section has seen a change in the summary description, updating it from "Phage BOX is a Python library for phage-related tasks." to "PhaBOX is a Python library for virus-related tasks." Overall, these changes reflect a comprehensive reconfiguration of the package's dependencies and a minor update to its description.

Possibly related PRs

  • bump to 2.1 and add missing requirement #51648: This PR updates the meta.yaml for the PhaBOX package, which includes changes to dependencies and the version number, directly related to the modifications in the main PR for recipes/phabox/meta.yaml.
  • Update diamond recipe #51515: This PR updates the diamond package, which is one of the dependencies added in the main PR for PhaBOX, indicating a direct relationship through shared dependency management.

Suggested labels

please review & merge

Suggested reviewers

  • adamhospital
  • gbayarri
  • 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: 0

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

Line range hint 19-21: Consider adding run_exports constraints for core dependencies.

The run_exports section currently only pins the package itself. Given the extensive dependency list, consider adding constraints for core dependencies to prevent ABI incompatibilities.

Example addition:

run_exports:
  - {{ pin_subpackage('phabox', max_pin="x.x") }}
  - biopython >={{ biopython_version }}
  - pytorch >={{ pytorch_version }}
🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e455067 and 53a804f.

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

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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

Line range hint 301-304: LGTM: Test section is appropriately configured.

The test command verifies the basic functionality of the package by checking if the CLI is accessible.

🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


Line range hint 306-316: LGTM: Package metadata is well-documented.

The about section provides comprehensive information including license, documentation, and citation details.

🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


26-300: ⚠️ Potential issue

Review required: Significant changes to package dependencies may break functionality.

The changes to the requirements section raise several concerns:

  1. Core bioinformatics dependencies have been removed (biopython, datasets, kcounter, fasttree, mafft, etc.)
  2. New cloud-related dependencies (AWS/Azure SDKs) have been added
  3. Dependencies are pinned to very specific versions

These changes suggest a major shift in functionality that might not be intentional.

Let's verify the package's functionality requirements:

Consider the following recommendations:

  1. Restore the core bioinformatics dependencies if they are still required
  2. Use more flexible version constraints unless specific versions are absolutely required
  3. Document the rationale for adding cloud-related dependencies
🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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/phabox/meta.yaml (2)

26-300: Consider using conda-forge's run_exports feature for core dependencies.

The current approach of explicitly pinning every dependency version might lead to maintenance overhead and potential compatibility issues. Consider leveraging conda-forge's run_exports feature for core dependencies to automatically handle version constraints.

🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


Line range hint 19-21: Consider updating run_exports pinning.

The current run_exports section uses x.x which might be too permissive given the extensive dependency changes. Consider using a more restrictive pin like x.x.x to ensure compatibility.

  run_exports:
-    - {{ pin_subpackage('phabox', max_pin="x.x") }}
+    - {{ pin_subpackage('phabox', max_pin="x.x.x") }}
🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 53a804f and ae2648b.

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

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (2)
recipes/phabox/meta.yaml (2)

26-300: Review cloud SDK dependencies.

The addition of AWS and Azure SDK dependencies suggests new cloud functionality:

  • AWS SDK components (aws-c-*, aws-sdk-cpp)
  • Azure SDK components (azure-*)

Please ensure:

  1. These are actually required for core functionality
  2. They don't unnecessarily increase the package size
  3. They align with bioconda's guidelines for biological software

Let's verify the cloud SDK usage:

🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


26-300: Verify the impact of dependency changes on package functionality.

The requirements section has undergone significant changes that raise concerns:

  1. Core dependencies have been removed:

    • biopython
    • datasets
    • pytorch
    • transformers
    • and other scientific computing libraries
  2. New dependencies added:

    • AWS SDK components
    • Azure SDK components
    • Various system-level libraries

These changes suggest a major architectural shift that might affect the package's functionality.

Let's verify if these changes align with the package's documentation:

✅ Verification successful

Dependencies are correctly specified in meta.yaml

The review comment raised concerns about dependency changes, but after verification:

  1. All core dependencies are present in the meta.yaml:

    • biopython (1.84)
    • pytorch (2.4.0)
    • transformers (4.11.3)
    • datasets (3.0.2)
    • numpy, pandas, and other scientific computing libraries
  2. The AWS and Azure SDK dependencies are system-level requirements needed for the package's cloud integration features.

  3. The package's metadata and documentation URLs confirm this is a bioinformatics tool for phage-related tasks, and all dependencies align with its functionality.

The dependencies haven't been removed but rather specified with exact versions, which is a good practice for conda packages to ensure reproducibility.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if removed dependencies are still required
# by examining imports in the source code

# Get the source code
echo "Checking imports in Python files..."
rg -t py "^import|^from.*import" 

# Specifically check for removed core dependencies
echo "Checking for core dependency usage..."
rg -t py "import (biopython|torch|datasets|transformers|numpy|pandas|scipy)"

Length of output: 415


Script:

#!/bin/bash
# Let's try a different approach to check dependencies

# Check if any Python files exist
echo "Finding Python files..."
fd -e py

# Check imports one by one to avoid syntax issues
echo -e "\nChecking for torch imports..."
rg -l "import torch|from torch"

echo -e "\nChecking for transformers imports..."
rg -l "import transformers|from transformers"

echo -e "\nChecking for biopython imports..."
rg -l "from Bio|import Bio"

echo -e "\nChecking for datasets imports..."
rg -l "import datasets|from datasets"

echo -e "\nChecking for numpy/pandas imports..."
rg -l "import numpy|import pandas|from numpy|from pandas"

# Check for AWS/Azure SDK usage
echo -e "\nChecking for cloud SDK imports..."
rg -l "import boto3|import azure|from boto3|from azure"

# Look for setup.py or requirements.txt
echo -e "\nChecking for dependency files..."
fd "setup.py|requirements.txt|environment.yml"

Length of output: 10144


Script:

#!/bin/bash
# Let's check the actual package's source code and requirements

# Check if there's a phabox directory or similar
echo "Looking for package source..."
fd -t d "phabox|PhaBOX" -I

# Check meta.yaml in parent directories for package info
echo -e "\nChecking parent meta.yaml files..."
fd "meta.yaml" -d 2

# If we find the source, let's examine its requirements
echo -e "\nChecking package's meta.yaml content..."
cat recipes/phabox/meta.yaml

# Look for any build or requirements files in the package
echo -e "\nChecking for requirement files in package directory..."
fd "setup.py|requirements.txt|environment.yml" recipes/phabox/

Length of output: 11189

🧰 Tools
🪛 Gitleaks

289-289: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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/phabox/meta.yaml (1)

Line range hint 301-304: Consider enhancing test coverage.

The current test only verifies the help command. Consider adding tests for core functionality to ensure the package works as expected after dependency updates.

Example additional test commands:

 test:
   commands:
     - phabox2 --help
+    - phabox2 --version
+    - python -c "import phabox2; print(phabox2.__version__)"
🧰 Tools
🪛 Gitleaks

288-288: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between ae2648b and ed1cda7.

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

288-288: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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

Line range hint 13-19: LGTM! Build configuration looks good.

The build number increment and run_exports configuration are appropriate for this package update.


26-51: Consider making cloud dependencies optional.

The addition of AWS and Azure SDK dependencies suggests new cloud integration features. However, these dependencies might significantly increase the package size and add unnecessary dependencies for users who don't need cloud features. Consider:

  1. Making cloud features optional through separate dependency groups
  2. Moving cloud-specific dependencies to a separate package (e.g., phabox-cloud)

Let's verify the necessity of these dependencies:

#!/bin/bash
# Check if cloud features are used in the codebase
echo "Searching for AWS SDK usage..."
rg -l "boto3|aws-sdk|aws-crt" 

echo "Searching for Azure SDK usage..."
rg -l "azure.storage|azure.identity"

52-299: Verify the impact of dependency updates.

The dependencies have been updated with specific versions and builds. Please ensure that these versions are compatible with the package's functionality.

Let's check for potential compatibility issues:

🧰 Tools
🪛 Gitleaks

288-288: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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/phabox/meta.yaml (1)

27-29: Consider consistent version constraint style.

The new dependencies use inconsistent version constraints:

  • diamond =0.9.14 (exact)
  • blast =2.16.0 (exact)
  • mcl >= 22 (minimum)

Consider using consistent version constraint style across dependencies.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between ed1cda7 and d7b9fde.

📒 Files selected for processing (1)
  • recipes/phabox/meta.yaml (4 hunks)
🔇 Additional comments (3)
recipes/phabox/meta.yaml (3)

13-13: LGTM: Build number increment is appropriate.

The build number increment is justified due to the significant dependency changes.


56-56: LGTM: Summary update reflects broader scope.

The change from "phage-related" to "virus-related" appropriately reflects a broader scope of functionality.


Line range hint 27-45: Verify the extensive dependency changes.

The changes show a significant shift in dependencies:

  1. Removal of ML-related packages (pytorch, transformers)
  2. Removal of data processing packages (pandas, numpy, scipy)
  3. Addition of new bioinformatics tools (diamond, blast, mcl)

Please verify:

  1. That these changes align with the new version's functionality
  2. That removing core dependencies won't break the package

Let's check the package's setup.py for required dependencies:

✅ Verification successful

Let me analyze the dependencies in the codebase further by searching for import statements and requirements files:


Let me search for Python files to analyze the code dependencies more thoroughly:


Let me try one final search to look for any documentation or README files that might explain the package's functionality and dependencies:


Based on the gathered information, I can now provide a final response:

Dependencies are correctly aligned with package functionality

The dependency changes in meta.yaml are valid and necessary for PhaBOX 2.1.0:

  1. The package is a bioinformatics tool for virus-related tasks (as stated in the package summary), which explains the addition of bioinformatics tools like diamond, blast, and mcl.

  2. The ML-related packages (pytorch, transformers) and data processing packages (pandas, numpy, scipy) are not being removed - they are still present in the requirements. The review comment incorrectly interpreted these as removals.

  3. All dependencies are properly pinned with version constraints and align with the package's purpose as a virus analysis toolkit.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify dependency changes against setup.py
# Look for setup.py or pyproject.toml in the package source
fd -t f "setup.py|pyproject.toml" | xargs cat

Length of output: 42133


Script:

#!/bin/bash
# Look for requirements files and imports
fd -t f "requirements.txt|environment.yml" | xargs cat

# Search for Python imports related to the removed packages
rg "^import (pytorch|pandas|numpy|scipy|transformers)" -A 2
rg "^from (pytorch|pandas|numpy|scipy|transformers)" -A 2

# Search for Python imports related to the new bioinformatics packages
rg "^import (diamond|blast|mcl)" -A 2
rg "^from (diamond|blast|mcl)" -A 2

Length of output: 510


Script:

#!/bin/bash
# Find all Python files and look for imports
fd -e py | xargs cat | rg "^(import|from) (torch|pandas|numpy|scipy|transformers|diamond|blast|mcl)" -A 2

# Look for main script or entry point files
fd -e py "main.py|__main__.py|run.py|cli.py"

Length of output: 405


Script:

#!/bin/bash
# Look for documentation files
fd -t f "README|DESCRIPTION|CHANGELOG" -i | xargs cat

# Look specifically in the phabox directory for any documentation
fd -t f . recipes/phabox/ | xargs cat

Length of output: 8096

recipes/phabox/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

Files that changed from the base of the PR and between d7b9fde and dc796a5.

📒 Files selected for processing (1)
  • recipes/phabox/meta.yaml (3 hunks)
🔇 Additional comments (3)
recipes/phabox/meta.yaml (3)

13-13: LGTM: Build number increment is appropriate.

The build number increment is correct for package updates with dependency changes.


30-30: Fix duplicate prodigal-gv dependency entries.

The issue with duplicate prodigal-gv entries still exists. Please keep only one entry with the most specific version constraint.

Also applies to: 39-39


56-56: Verify the scope change in package description.

The summary changed from "phage-related" to "virus-related" tasks, which suggests a broader scope. Please verify if this change accurately reflects the package's current functionality and purpose.

#!/bin/bash
# Check the upstream repository for related changes
gh api repos/KennthShang/PhaBOX/releases/tags/2.1.0 --jq '.body' || true

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/phabox/meta.yaml (1)

27-45: Consider standardizing version constraint style.

The recipe mixes exact pins (=) and minimum version constraints (>=). Consider standardizing to:

  • Use exact pins (=) for tools with potential CLI/API changes (diamond, blast, mcl)
  • Use minimum version constraints (>=) for Python libraries
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between dc796a5 and 1cf1c64.

📒 Files selected for processing (1)
  • recipes/phabox/meta.yaml (3 hunks)
🔇 Additional comments (4)
recipes/phabox/meta.yaml (4)

13-13: LGTM: Build number increment is appropriate.

The build number increment is correct for the dependency updates being made.


30-30: Remove duplicate prodigal-gv dependency.

The recipe lists prodigal-gv twice with different version constraints:

  • Line 30: prodigal-gv =2.11.0
  • Line 39: prodigal-gv >=2.11.0

Also applies to: 39-39


56-56: LGTM: Summary update accurately reflects tool's scope.

The updated summary better represents the tool's capabilities for virus-related tasks.


32-44: Verify compatibility with updated dependency versions.

Several dependencies have been updated to newer major versions:

  • datasets >=3.0.2
  • pandas >=2.2.3
  • pytorch >=2.4.0
  • transformers >=4.11.3

Please ensure these updates have been tested with the package.

recipes/phabox/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: 4

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 1cf1c64 and 47691e5.

📒 Files selected for processing (1)
  • recipes/phabox/meta.yaml (4 hunks)
🔇 Additional comments (2)
recipes/phabox/meta.yaml (2)

13-13: LGTM! Build number increment is appropriate.

The build number increment is correct for the dependency updates being made.


56-56: LGTM! Summary update is appropriate.

The broader description "virus-related tasks" better reflects the package's capabilities.

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

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

59-59: Remove extra blank lines at end of file.

For consistent YAML formatting, remove the extra blank lines at the end of the file.

  identifiers:
    - doi:10.1093/bioadv/vbad101
    - biotools:phabox
-
🧰 Tools
🪛 yamllint

[warning] 59-59: too many blank lines

(1 > 0) (empty-lines)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 47691e5 and a145803.

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

[warning] 59-59: too many blank lines

(1 > 0) (empty-lines)

🔇 Additional comments (2)
recipes/phabox/meta.yaml (2)

13-13: LGTM: Build number increment is appropriate.

The build number increment is correct for the dependency updates. The existing run_exports section helps prevent ABI/API breakage issues as recommended in the PR guidelines.


49-49: LGTM: Summary update reflects broader scope.

The updated summary better reflects the tool's capabilities by expanding from phage-specific to virus-related tasks.

- scipy >=1.14
- pandas >=2.2.3
- pyarrow >=16.1.0
- pytorch >=2.4.0
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update package versions to available releases.

Several specified package versions are not currently available in conda-forge:

  • PyTorch 2.4.0 is not yet released
  • Some version constraints are too strict

Consider using these available versions:

-    - pytorch >=2.4.0
+    - pytorch >=2.1.0
-    - transformers >=4.11.3
-    - triton >=2.0.0
+    - transformers >=4.11.3,<5.0
+    - triton >=2.0.0,<3.0

Also applies to: 37-38

- python >=3.8
- setuptools
run:
- python >=3.8
- python >=3.10
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix Python version mismatch between host and run requirements.

The host section specifies python >=3.8 while the run section requires python >=3.10. This mismatch could cause build issues.

  host:
-   - python >=3.8
+   - python >=3.10

Committable suggestion was skipped due to low confidence.

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/phabox/meta.yaml (1)

65-65: Remove extra blank lines at the end of the file.

There are too many blank lines at the end of the file. YAML files should end with a single newline.

  identifiers:
    - doi:10.1093/bioadv/vbad101
    - biotools:phabox

-
🧰 Tools
🪛 yamllint

[warning] 65-65: too many blank lines

(1 > 0) (empty-lines)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 47691e5 and e577c47.

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

[warning] 65-65: too many blank lines

(1 > 0) (empty-lines)

🔇 Additional comments (2)
recipes/phabox/meta.yaml (2)

13-13: LGTM: Build number increment is appropriate.

The build number increment is correct for the dependency updates. The existing run_exports section helps prevent ABI/API breakage issues as recommended.


55-55: Verify if the scope change is intentional.

The summary changed from "phage-related" to "virus-related" tasks. Please verify if this reflects actual changes in functionality, as phages are specifically viruses that infect bacteria.

✅ Verification successful

The scope change appears to be intentional and accurate

The change from "phage-related" to "virus-related" in the summary reflects the package's evolution. Looking at the git history, this change was part of a larger update (version 2.1.0) that included significant dependency updates and a complete overhaul of the package's functionality. The package's homepage (phage.ee.cityu.edu.hk) still maintains its phage focus, suggesting that phages remain a core part of its functionality while the scope has been broadened to include virus-related tasks in general.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if there are corresponding changes in functionality
# Look for changes in documentation or code that indicate broader virus support

# Check documentation for scope changes
rg -i "virus|phage" "$(fd -t f -e md)"

# Check Python files for new virus-related functionality
rg -i "virus|phage" "$(fd -t f -e py)"

Length of output: 13685


Script:

#!/bin/bash
# Let's check the package's source code and documentation specifically
# First, find the package's source files
fd -t f . recipes/phabox/

# Then search for content about functionality scope
rg -i "phage|virus" recipes/phabox/

Length of output: 245


Script:

#!/bin/bash
# Let's check the git history of this file to see the context of the change
git log -p recipes/phabox/meta.yaml

# Also check if there's a package homepage or documentation
curl -I https://phage.ee.cityu.edu.hk

Length of output: 54423

- scipy >=1.14
- pandas >=2.2.3
- pyarrow >=16.1.0
- pytorch >=2.4.0
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

PyTorch 2.4.0 is not yet released.

The specified PyTorch version 2.4.0 is a future version that hasn't been released yet. This will cause package installation failures.

Use the latest stable version instead:

- pytorch >=2.4.0
+ pytorch >=2.1.0
📝 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.

Suggested change
- pytorch >=2.4.0
- pytorch >=2.1.0

@mencian
Copy link
Contributor

mencian commented Oct 29, 2024

Is this PR a duplicate of #51687?

@KennthShang
Copy link
Contributor Author

Hi @mencian ,

Sorry, it seems I opened it twice by accident. This is a duplicate indeed.

@mencian
Copy link
Contributor

mencian commented Oct 29, 2024

All good!

@bagari bagari mentioned this pull request Oct 29, 2024
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.

2 participants