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.

Sign up for GitHub

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

Jump to bottom

add pantax #50983

Merged
merged 21 commits into from
Oct 10, 2024
Merged

add pantax #50983

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5268cf5
Create build.sh
zwh82 Sep 26, 2024
26c947f
Create meta.yaml
zwh82 Sep 26, 2024
3ea2ff2
Merge branch 'bioconda:master' into pantax
zwh82 Sep 26, 2024
cf7ac6c
Update meta.yaml
zwh82 Sep 26, 2024
dea4574
Update build.sh
zwh82 Sep 26, 2024
5b16876
Update meta.yaml
zwh82 Sep 26, 2024
fe19494
Update meta.yaml
zwh82 Sep 26, 2024
15006dd
Update build.sh
zwh82 Sep 27, 2024
fa9a0c5
Update meta.yaml
zwh82 Sep 27, 2024
dfb06a8
Update build.sh
zwh82 Sep 27, 2024
1bef665
Update recipes/pantax/meta.yaml
zwh82 Sep 27, 2024
3825b81
Update meta.yaml
zwh82 Oct 8, 2024
4a53b89
Update build.sh
zwh82 Oct 8, 2024
60d398c
Update build.sh
zwh82 Oct 9, 2024
80218be
Update meta.yaml
zwh82 Oct 9, 2024
10c9f3a
Update recipes/pantax/meta.yaml
zwh82 Oct 9, 2024
9b9dd6c
Update meta.yaml
zwh82 Oct 9, 2024
41021f1
Update meta.yaml
zwh82 Oct 9, 2024
83fa662
Update build.sh
zwh82 Oct 9, 2024
ac057c5
Update build.sh
zwh82 Oct 9, 2024
34e2e53
Update meta.yaml
zwh82 Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions recipes/pantax/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env bash

if [ "$(uname)" == "Darwin" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The meta.yaml says to skip OSX

cp -rf $SRC_DIR $PREFIX
else
martin-g marked this conversation as resolved.
Show resolved Hide resolved
zwh82 marked this conversation as resolved.
Show resolved Hide resolved
cd ${PREFIX}/gurobi11
${PYTHON} -m pip install gurobipy-11.0.2-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is not allowed to install dependencies with Pip like this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where does it install it, by the way ? It looks like it is no inside $PREFIX

Copy link
Contributor Author

@zwh82 zwh82 Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why? Are there any rules that restrict this installation behavior? I have already installed the artifact locally and tested it sucessfully, and PanTax is works well. Gurobipy, as a Python package, should be installed in the /path/to/env/lib/python3.10/site-packages/gurobipy. ${PREFIX} should be /path/to/env/bin，so it will certainly not be installed there.

Copy link
Contributor Author

@zwh82 zwh82 Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

About the question of vg version. I use vg version 1.52 instead of newer version to ensure the robustness and reproducibility of our tool. All my experiments are run on this version. In addition, I have a dependency PGGB. PGGB depends on vg version 1.40. Therefore, I cannot directly add vg to the dependency, otherwise the vg version 1.40 will be overwritten by the new version of vg.

rm gurobipy-11.0.2-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl
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

Improve error handling and Python version flexibility.

  1. Add error handling for the cd command:
-cd ${PREFIX}/gurobi11
+cd ${PREFIX}/gurobi11 || exit 1
  1. Make the Python wheel installation more flexible:
-${PYTHON} -m pip install gurobipy-11.0.2-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl
+${PYTHON} -m pip install gurobipy-11.0.2-cp${PYTHON_VERSION/./}-cp${PYTHON_VERSION/./}-manylinux2014_x86_64.manylinux_2_17_x86_64.whl

This assumes that the PYTHON_VERSION environment variable is available and set correctly. If it's not, you may need to extract it from ${PYTHON} or set it explicitly.

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 Shellcheck

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

(SC2164)


mkdir -p ${PREFIX}/bin/tools
cd ${PREFIX}/vg
cp vg* ${PREFIX}/bin/tools/vg
chmod +x ${PREFIX}/bin/tools/vg
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

Improve error handling and file copying specificity.

  1. Add error handling for the cd command:
-cd ${PREFIX}/vg
+cd ${PREFIX}/vg || exit 1
  1. Consider being more specific when copying files to avoid including unnecessary files:
-cp vg* ${PREFIX}/bin/tools/vg
+cp vg vg_* ${PREFIX}/bin/tools/

This assumes that the main executable is named vg and any related files start with vg_. Adjust the pattern as needed for your specific use case.

📝 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
mkdir -p ${PREFIX}/bin/tools
cd ${PREFIX}/vg
cp vg* ${PREFIX}/bin/tools/vg
chmod +x ${PREFIX}/bin/tools/vg
mkdir -p ${PREFIX}/bin/tools
cd ${PREFIX}/vg || exit 1
cp vg vg_* ${PREFIX}/bin/tools/
chmod +x ${PREFIX}/bin/tools/vg
🧰 Tools
🪛 Shellcheck

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

(SC2164)


cd $SRC_DIR/scripts
chmod +x pantax
chmod +x data_preprocessing
cp $SRC_DIR/scripts/pantax $SRC_DIR/scripts/data_preprocessing ${PREFIX}/bin
cp $SRC_DIR/scripts/*py ${PREFIX}/bin
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

Improve error handling and file copying specificity.

  1. Add error handling for the cd command:
-cd $SRC_DIR/scripts
+cd $SRC_DIR/scripts || exit 1
  1. Consider being more specific when copying Python files to avoid including unnecessary files:
-cp $SRC_DIR/scripts/*py ${PREFIX}/bin
+cp $SRC_DIR/scripts/{pantax.py,data_preprocessing.py} ${PREFIX}/bin

Replace pantax.py and data_preprocessing.py with the actual names of the Python scripts that need to be copied. This approach ensures that only the necessary files are included in the installation.

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 Shellcheck

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

(SC2164)


cd $SRC_DIR/tools/fastix
cargo install fastix --root ./
zwh82 marked this conversation as resolved.
Show resolved Hide resolved
cp $SRC_DIR/tools/fastix/bin/fastix ${PREFIX}/bin/tools
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

Improve error handling and add Cargo availability check.

  1. Add error handling for the cd command:
-cd $SRC_DIR/tools/fastix
+cd $SRC_DIR/tools/fastix || exit 1
  1. Add a check for Cargo availability before attempting to install Fastix:
+if ! command -v cargo &> /dev/null; then
+    echo "Cargo is not installed. Please install Rust and Cargo to build Fastix."
+    exit 1
+fi
cargo install fastix --root ./

This change ensures that the script fails gracefully if Cargo is not available on the system.

📝 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
cd $SRC_DIR/tools/fastix
cargo install fastix --root ./
cp $SRC_DIR/tools/fastix/bin/fastix ${PREFIX}/bin/tools
cd $SRC_DIR/tools/fastix || exit 1
if ! command -v cargo &> /dev/null; then
echo "Cargo is not installed. Please install Rust and Cargo to build Fastix."
exit 1
fi
cargo install fastix --root ./
cp $SRC_DIR/tools/fastix/bin/fastix ${PREFIX}/bin/tools
🧰 Tools
🪛 Shellcheck

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

(SC2164)

fi
57 changes: 57 additions & 0 deletions recipes/pantax/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{% set name = "pantax" %}
{% set version = "1.0.1" %}

package:
name: {{ name }}
version: {{ version }}

source:
- url: https://github.com/LuoGroup2023/PanTax/releases/download/v1.0.1/pantax.tar.gz
sha256: 93f39878f15cb76d40a80e8ae1b3521f1a4bc799c0cc4519c3df9c4ea9c4b9ba

- url: https://files.pythonhosted.org/packages/07/0f/6039cf6e22f9cbec57cdedff103949e4856c18fd2cb714bc70cdc8b52941/gurobipy-11.0.2-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl
Copy link
Member

Choose a reason for hiding this comment

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

Please require this as a dependency and do not pip install them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gurobipy can be install with conda install gurobi::gurobi and pip. But I don't know how to set channels gurobi to install it.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can use only dependencies from conda-forge and bioconda channels. You cannot install from third-party channels and/or Pip

sha256: 164e72462c2f1a705fcec73acc060aa639e745827bfe877714ff7b2ccabc5237
folder: gurobi11

- url: https://github.com/vgteam/vg/releases/download/v1.59.0/vg
sha256: a2270237c8541867ac345ad924c1641881bc9fd60be157a6d3d29fa910aacfc2
folder: vg
Copy link
Member

Choose a reason for hiding this comment

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

Can you instead require VG in the requirements section?

Copy link
Contributor Author

@zwh82 zwh82 Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is impossible. The requirement PGGB works with vg 1.40, but I need updated versions (vg 1.52) in other parts of my codes.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://anaconda.org/bioconda/vg provides a newer version


build:
skip: True # [osx]
number: 0
run_exports:
- {{ pin_subpackage(name, max_pin="x.x") }}
zwh82 marked this conversation as resolved.
Show resolved Hide resolved
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 pin_subpackage directive and remove trailing space.

The build section is generally well-defined. However, consider the following points:

  1. The pin_subpackage directive uses "x.x", which might be too strict for a 1.x version. For version 1.x, using "x" is typically sufficient unless you expect API breaks in minor versions.
  2. There's a trailing space on line 16 that should be removed.

Consider updating the run_exports section as follows:

  run_exports:
    - {{ pin_subpackage(name, max_pin="x") }}

This change allows for minor version updates while still maintaining compatibility.

🧰 Tools
🪛 yamllint

[error] 16-16: trailing spaces

(trailing-spaces)


requirements:
build:
- {{ compiler('rust') }}
host:
- python ==3.10
- pip
run:
- python =3.10
martin-g marked this conversation as resolved.
Show resolved Hide resolved
- samtools =1.19.2
- bcftools =1.19
- htslib =1.19.1
- pggb =0.5.4
- graphaligner =1.0.17
- h5py =3.10.0
- pandas =2.2.0
- tqdm =4.66.1
- numpy =1.26.3
- networkx =3.2.1
- time =1.8

test:
commands:
- pantax -h
zwh82 marked this conversation as resolved.
Show resolved Hide resolved

about:
home: https://github.com/LuoGroup2023/PanTax
license: GPL-3.0
zwh82 marked this conversation as resolved.
Show resolved Hide resolved
summary: Strain-level taxonomic classification of metagenomic data using pangenome graphs

extra:
skip-lints:
- should_be_noarch_python
Loading