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

GenEra Conda Recipes Submission #51721

Merged
merged 52 commits into from
Nov 25, 2024

Conversation

AnupamGautam
Copy link
Contributor

Dear Administrator,

Please find attached the Conda recipes for GenEra. Let us know if any further modifications are required. We look forward to your feedback.

Best regards,
Anupam

Copy link
Contributor

coderabbitai bot commented Oct 28, 2024

📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request introduces two significant components for the genera package. First, a new script named build.sh is added to automate the installation of various executables related to the project. The script ensures that key scripts are made executable, moves the Run_abSENSE.py script from the abSENSE directory into the current directory, and establishes a Conda binary path for the executables. Second, a meta.yaml file is created for the genera package, version 1.4.2, containing essential metadata such as the source repository URL, versioning, SHA256 checksum for integrity, and build number. This file outlines the required dependencies for both host and runtime environments, which include specific Python and R packages, as well as other necessary tools for the package's functionality. The meta.yaml also provides additional information, including the package's homepage and license type.

Possibly related PRs

  • Add recipe for consensify #50969: The build.sh script in this PR automates the installation process, similar to the build.sh in the main PR, which also focuses on installation automation for executables.
  • recipe for pgrc #50973: The build.sh script for pgrc automates the build process, akin to the build.sh in the main PR, which also sets up an installation environment.
  • add recipe for sizemeup #51033: The build.sh script for sizemeup is designed to facilitate installation, paralleling the installation automation focus of the build.sh in the main PR.
  • add gfainject #51287: The build.sh script for gfainject automates the build process, similar to the automation focus in the main PR's build.sh.
  • Add seqspec #51326: The build.sh script for seqspec is aimed at installation automation, reflecting the same purpose as the build.sh in the main PR.
  • Add meta_eukaryome_detect #51328: The build.sh script for meta_eukaryome_detect automates the build process, similar to the automation in the main PR's build.sh.
  • add meta #51349: The build.sh script for meta automates the installation process, akin to the build.sh in the main PR.
  • Add svtopo recipe #51665: The build.sh script for svtopo automates the installation, similar to the automation focus of the build.sh in the main PR.
  • Add libgff recipe #51706: The build.sh script for libgff automates the build process, reflecting the same automation goal as the build.sh in the main PR.
  • Add amplici package in bioconda #51847: The build.sh script for amplici is designed for installation automation, paralleling the automation focus of the build.sh in the main PR.

Suggested reviewers

  • martin-g

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 (4)
recipes/genera/build.sh (3)

1-4: Add PREFIX variable validation.

The script assumes PREFIX is set but should validate it since it's critical for Conda builds.

Add this check after line 4:

 set -ex

+# Validate PREFIX environment variable
+if [ -z "${PREFIX}" ]; then
+    echo "Error: PREFIX environment variable is not set"
+    exit 1
+fi

21-28: Add error handling for file moving operations.

The current implementation doesn't verify successful file moves.

Suggested improvement:

-mv genEra "$CONDABIN"
-mv Erassignment "$CONDABIN"
-mv Run_abSENSE.py "$CONDABIN"
-mv hmmEra "$CONDABIN"
-mv tree2ncbitax "$CONDABIN"
-mv FASTSTEP3R "$CONDABIN"
-mv test_installation.sh "$CONDABIN"
+for file in genEra Erassignment Run_abSENSE.py hmmEra tree2ncbitax FASTSTEP3R test_installation.sh; do
+    if ! mv "$file" "$CONDABIN/"; then
+        echo "Error: Failed to move $file to $CONDABIN"
+        exit 1
+    fi
+done

30-30: Add installation verification step.

Consider adding a verification step to ensure all executables are properly installed.

Add after the echo:

 echo "Installation complete. Executables have been moved to ${CONDABIN}."
+# Verify installation
+echo "Verifying installation..."
+for file in genEra Erassignment Run_abSENSE.py hmmEra tree2ncbitax FASTSTEP3R test_installation.sh; do
+    if [ ! -x "$CONDABIN/$file" ]; then
+        echo "Error: $file not found or not executable in $CONDABIN"
+        exit 1
+    fi
+done
+echo "Verification complete. All executables are properly installed."
recipes/genera/meta.yaml (1)

30-33: Add additional package metadata.

Please enhance the metadata section with:

  1. License file specification
  2. License family
  3. More detailed description
 about:
   home: https://github.com/josuebarrera/GenEra.git
   license: GPL-3.0
+  license_family: GPL
+  license_file: LICENSE
   summary: "Uncovering gene-family founder events during major evolutionary transitions in animals, plants and fungi using GenEra"
+  description: |
+    GenEra is a comprehensive tool for analyzing gene-family founder events
+    during major evolutionary transitions. It provides functionality for
+    studying evolutionary patterns in animals, plants, and fungi through
+    various computational and statistical approaches.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 3f0cfa7 and 1c55c53.

📒 Files selected for processing (2)
  • recipes/genera/build.sh (1 hunks)
  • recipes/genera/meta.yaml (1 hunks)
🔇 Additional comments (2)
recipes/genera/build.sh (1)

14-19: LGTM: Proper Conda binary path handling.

The script correctly uses PREFIX and ensures the bin directory exists.

recipes/genera/meta.yaml (1)

10-11: Verify build script inclusion.

The AI summary mentions a build.sh script, but it's not referenced in the meta.yaml. Please ensure the build script is included in the recipe directory.

✅ Verification successful

Build script is present and properly configured

The build.sh script exists in the recipe directory and contains appropriate installation steps including:

  • Setting executable permissions
  • Cloning required dependencies
  • Installing executables to the correct conda binary path
  • Using conda environment variables (PREFIX)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if build.sh exists in the recipe directory
fd -t f "build.sh" recipes/genera/

Length of output: 58


Script:

#!/bin/bash
# Let's check the content of build.sh to verify it's not empty and properly configured
cat recipes/genera/build.sh

Length of output: 784

Comment on lines 9 to 12
# Clone the secondary repository and move the main script
git clone https://github.com/caraweisman/abSENSE.git
mv abSENSE/Run_abSENSE.py .
chmod +x Run_abSENSE.py
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 28, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Remove git clone operation from build script.

Downloading dependencies during build time violates Conda packaging best practices and makes builds non-reproducible. Instead:

  1. Include abSENSE as a source in meta.yaml
  2. Pin the specific version/commit of abSENSE

Move this dependency to meta.yaml:

source:
  - url: [original genera url]
    sha256: [original sha]
  - url: https://github.com/caraweisman/abSENSE/archive/[specific-commit].tar.gz
    sha256: [checksum]
    folder: abSENSE

Then in build.sh, replace the clone with:

-git clone https://github.com/caraweisman/abSENSE.git
-mv abSENSE/Run_abSENSE.py .
+mv abSENSE/Run_abSENSE.py .

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right that the git clone command should be removed, but instead of adding the missing dependency as a source, a separate recipe should be created for the dependency in a new PR and then the dependencies updated in this recipe.

Copy link
Contributor

Choose a reason for hiding this comment

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

@aliciaaevans, understood. Thank you for the clarification. We'll adjust our recommendations accordingly.

🐰🤔


✏️ Learnings added
Learnt from: aliciaaevans
PR: bioconda/bioconda-recipes#51721
File: recipes/genera/build.sh:9-12
Timestamp: 2024-10-29T13:49:22.049Z
Learning: When reviewing Bioconda recipes, if a recipe depends on a missing dependency, it's preferable to create a separate recipe for the dependency in a new PR and update the dependencies in the main recipe accordingly, instead of adding the dependency as a source in the same recipe.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Contributor Author

@AnupamGautam AnupamGautam Oct 29, 2024

Choose a reason for hiding this comment

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

Hi @aliciaaevans, thank you for the review. This particular script doesn’t have a release version as I believe it’s part of a publication’s codebase. I’ve submitted a request on their GitHub for a release, but if they don’t create one, would it be possible to proceed with git clone despite the recommendation to avoid it?

caraweisman/abSENSE#3

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the required recipe for abSENSE (it has been merged https://anaconda.org/bioconda/absense), removed the corresponding section from GenEra, and made the necessary adjustments

recipes/genera/build.sh Show resolved Hide resolved
Comment on lines 14 to 15
host:
- python=3.7.12
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 Python version constraint.

The Python version is pinned to an old version (3.7.12). Consider using a more recent version or a minimum version constraint.

   host:
-    - python=3.7.12
+    - python >=3.8
+    - pip
+    - setuptools
📝 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
host:
- python=3.7.12
host:
- python >=3.8
- pip
- setuptools

Comment on lines 5 to 8
source:
git_url: https://github.com/josuebarrera/GenEra.git
git_tag: v1.4.2
sha256: d56c5f7430a33bf781a42b325c0cd813e862b06e14d486c411ba5909251ef206
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

Use release tarball URL instead of git repository.

For better reproducibility and faster downloads, bioconda prefers using release tarballs instead of git repositories.

 source:
-  git_url: https://github.com/josuebarrera/GenEra.git
-  git_tag: v1.4.2
+  url: https://github.com/josuebarrera/GenEra/archive/refs/tags/v1.4.2.tar.gz
   sha256: d56c5f7430a33bf781a42b325c0cd813e862b06e14d486c411ba5909251ef206
📝 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
source:
git_url: https://github.com/josuebarrera/GenEra.git
git_tag: v1.4.2
sha256: d56c5f7430a33bf781a42b325c0cd813e862b06e14d486c411ba5909251ef206
source:
url: https://github.com/josuebarrera/GenEra/archive/refs/tags/v1.4.2.tar.gz
sha256: d56c5f7430a33bf781a42b325c0cd813e862b06e14d486c411ba5909251ef206

Comment on lines 16 to 28
run:
- python=3.7.12
- diamond=2.1.10
- mmseqs2=14.7e284
- foldseek=3.915ef7d
- scipy=1.7.3
- r-seqinr=4.2_16
- r-optparse=1.7.3
- r-bio3d=2.4_3
- r-tidyverse=1.3.2
- r-phytools=0.6_99
- orthofinder=2.5.5
- ncbitax2lin=2.3.2
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Review dependency version constraints.

Some suggestions for the runtime dependencies:

  1. Use minimum version constraints (>=) instead of exact pins where possible
  2. Python version should match host requirements
  3. Consider adding commonly required R dependencies like r-base
   run:
-    - python=3.7.12
+    - python >=3.8
+    - r-base
     - diamond=2.1.10
     - mmseqs2=14.7e284
     - foldseek=3.915ef7d
-    - scipy=1.7.3
+    - scipy >=1.7.3
     - r-seqinr=4.2_16
     - r-optparse=1.7.3
     - r-bio3d=2.4_3
     - r-tidyverse=1.3.2
     - r-phytools=0.6_99
     - orthofinder=2.5.5
     - ncbitax2lin=2.3.2

Committable suggestion was skipped due to low confidence.

recipes/genera/build.sh Outdated Show resolved Hide resolved
recipes/genera/meta.yaml Outdated Show resolved Hide resolved
Comment on lines 8 to 10
- url: https://github.com/caraweisman/abSENSE/archive/refs/tags/v1.0.0.tar.gz
sha256: a99090c10a66afae1fbad38d79d228199e9fbdae5173e71ad5d84b4c958dda54
folder: abSENSE
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this is not a separate recipe?

Copy link
Contributor Author

@AnupamGautam AnupamGautam Nov 18, 2024

Choose a reason for hiding this comment

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

Dear @mfansler,

I have implemented your suggestions and added a new test as well. Regarding the tool abSENSE, I have reached out to the author to confirm their interest in having a Conda package. If they express interest, I can create a PR for the package.

For now, we can proceed like this, and if author want I can create a PR later ,and update this package also. Could you please approve the workflow so that it can start? Thank you so much for your help!

Best regards,
Anupam

Copy link
Member

Choose a reason for hiding this comment

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

As far as I see, abSENSE has no licensing declared, which is prohibitive to bundling it for Bioconda. Please ask the author to declare a license and include that in a release.

Once there is a license it should be moved to a separate package here. Note that the author does not have to be a maintainer here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dear @mfansler,

I had reached out to the authors a few weeks ago regarding the LICENSE, release, and a potential Conda package. Since then, they have created a release for us, but I assume a Conda package may not be something they are prioritizing.

This code is part of a published open-access paper, and I believe it is included in the supplementary materials. In this case, would a license still be required for the Conda package?

Here’s the relevant issue for reference: abSENSE Issue #3.

Here's the paper: https://journals.plos.org/plosbiology/article?id=10.1371/journal.pbio.3000862

They also mention in paper:"An implementation of our method, with all raw data and results presented here, is freely available as source code at github.com/caraweisman/abSENSE and as a web server at eddylab.org/abSENSE."
Thank you for your guidance!

Best regards,
Anupam

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the additional information!

While the article itself is CC-BY-4.0, a copy of the code does not appear deposited with the article and thus it is not covered. If they want to extend exactly the same license, they could put a copy of the CC-BY-4.0 text in the repository somewhere.

Otherwise, unfortunately "open-source" doesn't translate to permission to redistribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @mfansler,

I have reached out to the author again for clarification. Once they respond, I will proceed as suggested. However, if they decide against having a Conda package but provide a license, I can move forward using the current approach?

Thank you for your support!

Additionally, could you please help me start the workflow for my other recipe? It doesn’t seem to start automatically. Your assistance would be greatly appreciated.: TIPP_plastid conda recipe.

Best regards,
Anupam

Copy link
Contributor Author

@AnupamGautam AnupamGautam Nov 18, 2024

Choose a reason for hiding this comment

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

Dear @mfansler,

The authors have added a license file to their repository and released a new version. I’ve updated the meta.yaml file to include this latest release. While they may still need time to decide whether they want an official Conda package, I can create one for them later if requested and make the necessary updates.

For now, we can proceed with the current setup. Could you help initiate the workflow and assist in merging this?

Thank you for your continued support!

Best regards,
Anupam

Copy link
Member

Choose a reason for hiding this comment

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

That's great!

As a Conda Forge reviewer, I have the perspective that vendoring other package code directly into bin is a no-go. The reason is that, should the other package ever be moved to its own recipe, then they would clobber each other. There are post hoc ways around this (e.g., run_constrained), but it's preferable to avoid this. There are two ways one can do so, either:

  1. Package the code separately. While it is courteous to contact upstream developers about wanting to distribute as a Conda package, this is not required. There is no requirement to get personal approval, rather the license itself is the approval.

  2. Refactor so that the vendored code is within a "namespace" unique to your package. This is much clearer how to do it when one has a proper package (i.e., Python, R packages). For scripts such as being delivered here, it is less clear what the best practice is for namespacing.

I recommend option 1, moving it to a separate package. Maybe others who work Bioconda recipes more regularly can weigh in (CC @aliciaaevans @martin-g). It could be that Bioconda is less strict about this and I'm unnecessarily bothering you with Conda Forge standards (if so, apologies!).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dear @mfansler,

Thank you so much for your help—I really appreciate it. Let's wait and see what others have to say. If needed, I can also include this script in our package or create a dedicated Conda package for it.

Best regards,
Anupam

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

10-12: Consider creating a separate recipe for abSENSE in the future.

While including abSENSE directly in this recipe works for now, creating a separate recipe would improve maintainability and allow independent versioning. This would also make it easier for other packages to depend on abSENSE if needed.


37-41: Enhance test coverage.

Consider adding tests for all installed executables to ensure they are properly installed and accessible.

 test:
   commands:
     - genEra -h
     - hmmEra -h
     - test_installation.sh -h
+    - Run_abSENSE.py -h  # Test abSENSE script
+    # Add tests for any other executables installed by build.sh
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 0203536 and f183729.

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

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

(syntax)

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

1-6: LGTM! Version templating implemented correctly.

The package metadata section follows best practices with proper version templating.

🧰 Tools
🪛 yamllint

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

(syntax)


43-47: LGTM! About section is complete and follows best practices.

The about section contains all required information with proper SPDX license identifier.

recipes/genera/meta.yaml Show resolved Hide resolved
Comment on lines 21 to 35
host:
- python >=3.8,<3.9
run:
- python >=3.8,<3.9
- diamond >=2.1.10
- mmseqs2 =14.7e284
- foldseek =3.915ef7d
- scipy =1.7.3
- r-seqinr =4.2_16
- r-optparse =1.7.3
- r-bio3d =2.4_3
- r-tidyverse =1.3.2
- r-phytools >=0.6_99
- orthofinder =2.5.5
- ncbitax2lin =2.3.2
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Revise dependency specifications.

Several adjustments needed to align with Conda best practices:

  1. Remove upper bound on Python version
  2. Add missing build dependencies
  3. Use minimum version constraints (>=) instead of exact pins where possible
 requirements:
   host:
-    - python >=3.8,<3.9
+    - python >=3.8
+    - pip
+    - setuptools
   run:
-    - python >=3.8,<3.9
+    - python >=3.8
     - diamond >=2.1.10
-    - mmseqs2 =14.7e284
+    - mmseqs2 >=14.7e284
-    - foldseek =3.915ef7d
+    - foldseek >=3.915ef7d
-    - scipy =1.7.3
+    - scipy >=1.7.3
-    - r-seqinr =4.2_16
+    - r-seqinr >=4.2_16
-    - r-optparse =1.7.3
+    - r-optparse >=1.7.3
-    - r-bio3d =2.4_3
+    - r-bio3d >=2.4_3
-    - r-tidyverse =1.3.2
+    - r-tidyverse >=1.3.2
     - r-phytools >=0.6_99
-    - orthofinder =2.5.5
+    - orthofinder >=2.5.5
-    - ncbitax2lin =2.3.2
+    - ncbitax2lin >=2.3.2
📝 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
host:
- python >=3.8,<3.9
run:
- python >=3.8,<3.9
- diamond >=2.1.10
- mmseqs2 =14.7e284
- foldseek =3.915ef7d
- scipy =1.7.3
- r-seqinr =4.2_16
- r-optparse =1.7.3
- r-bio3d =2.4_3
- r-tidyverse =1.3.2
- r-phytools >=0.6_99
- orthofinder =2.5.5
- ncbitax2lin =2.3.2
host:
- python >=3.8
- pip
- setuptools
run:
- python >=3.8
- diamond >=2.1.10
- mmseqs2 >=14.7e284
- foldseek >=3.915ef7d
- scipy >=1.7.3
- r-seqinr >=4.2_16
- r-optparse >=1.7.3
- r-bio3d >=2.4_3
- r-tidyverse >=1.3.2
- r-phytools >=0.6_99
- orthofinder >=2.5.5
- ncbitax2lin >=2.3.2

Copy link
Member

Choose a reason for hiding this comment

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

I agree with the bot here. Are the exact specifications really necessary?

The run_exports of other Bioconda packages will add upper bounds that help constrain co-installation with too new versions in the future.

Also, Python 3.8 has reached EOL, so Python 3.9 would make more sense as a minimum.

The pip and setuptools suggested by the bot can be ignored - they aren't relevant since this only copies scripts.

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

37-41: Enhance test coverage.

Consider adding tests for all executables that are installed in the bin/ directory. This ensures that all components are properly installed and accessible.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f183729 and f4babdd.

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

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

(syntax)

🔇 Additional comments (4)
recipes/genera/meta.yaml (4)

1-6: LGTM! Package metadata follows best practices.

The version is properly templated and the package name follows conventions.

🧰 Tools
🪛 yamllint

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

(syntax)


17-18: Remove unnecessary run_exports section.


21-35: Revise dependency specifications.


43-47: LGTM! About section follows best practices.

The metadata is complete with proper SPDX license identifier and descriptive summary.

Comment on lines 10 to 12
- url: https://github.com/caraweisman/abSENSE/archive/refs/tags/v1.0.1.tar.gz
sha256: 853418e55c012c0dd409c97a70bbc9ecb69dfa7ccc94ec0f54d54ff99ec0e9f0
folder: abSENSE
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

Critical: Licensing issue with abSENSE component.

The abSENSE component currently lacks a proper license declaration. While the associated paper is open-access, the code itself needs an explicit license for redistribution in bioconda. This component should either:

  1. Be moved to a separate recipe once properly licensed, or
  2. Be removed until licensing is clarified

Would you like assistance in:

  1. Creating a separate recipe for abSENSE once licensed?
  2. Modifying this recipe to work without abSENSE?

@AnupamGautam
Copy link
Contributor Author

Dear @mfansler and @aliciaaevans,

I have added the required recipe for abSENSE (it has been merged https://anaconda.org/bioconda/absense), removed the corresponding section from GenEra, and made the necessary adjustments. The updated recipe has passed all checks.

I believe this issue can now be resolved and merged. Please let me know if further changes are needed.

Best regards,
Anupam

recipes/genera/meta.yaml Outdated Show resolved Hide resolved
Comment on lines 21 to 35
host:
- python >=3.8,<3.9
run:
- python >=3.8,<3.9
- diamond >=2.1.10
- mmseqs2 =14.7e284
- foldseek =3.915ef7d
- scipy =1.7.3
- r-seqinr =4.2_16
- r-optparse =1.7.3
- r-bio3d =2.4_3
- r-tidyverse =1.3.2
- r-phytools >=0.6_99
- orthofinder =2.5.5
- ncbitax2lin =2.3.2
Copy link
Member

Choose a reason for hiding this comment

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

I agree with the bot here. Are the exact specifications really necessary?

The run_exports of other Bioconda packages will add upper bounds that help constrain co-installation with too new versions in the future.

Also, Python 3.8 has reached EOL, so Python 3.9 would make more sense as a minimum.

The pip and setuptools suggested by the bot can be ignored - they aren't relevant since this only copies scripts.

@AnupamGautam
Copy link
Contributor Author

Dear @mfansler,

I have adjusted the flexibility wherever possible. However, Python 3.8 (or versions below 3.9) is currently required as abSENSE is not compatible with Python >=3.9. Until the author provides an update, we will need to maintain this version requirement. Once they implement compatibility, I will update the recipe accordingly.

Regarding other dependencies, some flags are only available in specific versions, which necessitates retaining version-specific requirements for now.

Best regards,
Anupam

Copy link
Member

@mfansler mfansler left a comment

Choose a reason for hiding this comment

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

Okay, let's merge.


Note

I have adjusted the flexibility wherever possible. However, Python 3.8 (or versions below 3.9) is currently required as abSENSE is not compatible with Python >=3.9. Until the author provides an update, we will need to maintain this version requirement. Once they implement compatibility, I will update the recipe accordingly.

Dependencies do not usually dictate the Python compatibility of a package. While that might restrict the environments in which it can be installed, the Python compatibility should be intrinsic to the package code (e.g., what syntactic features it uses). In this line, were an abSENSE that is Python v3.9+ released, one would immediately have the ability to co-install with it, rather than having to build and upload new artifacts.

However, I won't insist on this.

@AnupamGautam
Copy link
Contributor Author

Dear @mfansler,

Thank you so much for your approval and support; I truly appreciate it. I believe I’ll need to wait for @aliciaaevans before the changes can be merged.

Best regards,
Anupam

@aliciaaevans
Copy link
Contributor

@BiocondaBot please fetch artifacts

@BiocondaBot
Copy link
Collaborator

Package(s) built are ready for inspection:

Arch Package Zip File / Repodata CI Instructions
noarch genera-1.4.2-py38hdfd78af_0.tar.bz2 noarch.zip GitHub Actions
showYou may also use conda to install after downloading and extracting the zip file. conda install -c ./packages <package name>

Docker image(s) built:

Package Tag CI Install with docker
genera 1.4.2--py38hdfd78af_0 GitHub Actions
showImages are in the linux-64 zip file above.gzip -dc images/genera---1.4.2--py38hdfd78af_0.tar.gz | docker load

@aliciaaevans aliciaaevans merged commit 801fad7 into bioconda:master Nov 25, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants