Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Instanovo #51796

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Changes from 12 commits
Commits
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
66 changes: 66 additions & 0 deletions recipes/instanovo/meta.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{% set name = "instanovo" %}
{% set version = "1.0.0" %}

package:
name: {{ name|lower }}
version: {{ version }}

source:
url: https://pypi.org/packages/source/{{ name[0] }}/{{ name }}/instanovo-{{ version }}.tar.gz
sha256: fd9cfc377d9f8da5272f96b2eb4c14c08b579d7a65466aa402601ec6c4b42672

build:
noarch: python
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
number: 0
run_exports:
- {{ pin_subpackage(name | lower, max_pin="x.x") }}
Comment on lines +16 to +17
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

Remove unnecessary run_exports section

The run_exports section is typically used for C/C++ libraries to ensure ABI compatibility. For a pure Python package (noarch: python), this section is unnecessary and could cause unintended version pinning issues.

Apply this diff to remove the section:

 build:
   noarch: python
   script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
   number: 0
-  run_exports:
-    - {{ pin_subpackage(name | lower, max_pin="x.x") }}
📝 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
run_exports:
- {{ pin_subpackage(name | lower, max_pin="x.x") }}
build:
noarch: python
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
number: 0


Comment on lines +12 to +18
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

Remove unnecessary run_exports section.

The run_exports section is typically used for C/C++ libraries to ensure ABI compatibility. For a pure Python package (noarch: python), this section is unnecessary and could cause unintended version pinning issues.

Apply this diff to remove the section:

 build:
   noarch: python
   script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
   number: 0
-  run_exports:
-    - {{ pin_subpackage(name | lower, max_pin="x.x") }}
📝 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
build:
noarch: python
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
number: 0
run_exports:
- {{ pin_subpackage(name | lower, max_pin="x.x") }}
build:
noarch: python
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
number: 0


requirements:
host:
- python >=3.10,<3.12
- setuptools >=69.1.1
- pip
run:
- click >=8.1.7
- datasets >=3.0.1
- hydra-core >=1.3.2
- jaxtyping >=0.2.34
- jiwer >=3.0.4
- matchms >=0.27.0
hechth marked this conversation as resolved.
Show resolved Hide resolved
- neptune >=1.12.0
- numpy >=1.23,<1.27
hechth marked this conversation as resolved.
Show resolved Hide resolved
- omegaconf >=2.3.0
- pandas >=2.2.3
- polars >=1.9.0
- pyopenms >=3.2.0
- python-dotenv >=1.0.1
- pytorch-lightning >=2.4.0
- s3fs >=2024.6.1
- scikit-learn >=1.5.2
- spectrum_utils >=0.4.2
- tensorboard >=2.18.0
- pytorch-gpu >=2.4.1
- tqdm >=4.66.5
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Package availability and PyTorch specification need attention

The verification confirms critical issues with the package requirements:

  • The following packages are not available in conda-forge:

    • matchms
    • spectrum_utils
    • pyopenms
    • pytorch-gpu (this specific name is not a valid conda-forge package)
  • For PyTorch:

    • Neither pytorch-gpu nor the main pytorch package exists as conda-forge feedstocks
    • Only pytorch-cpu is found in conda-forge
    • The current specification pytorch-gpu >=2.4.1 will not work

Recommendations:

  1. Use PyPI versions for matchms, spectrum_utils, and pyopenms by adding them to pip requirements
  2. Replace pytorch-gpu with the correct package name from the channel where you intend to source it (likely pytorch from the pytorch channel)
🔗 Analysis chain

Address dependency availability and specification issues.

  1. Several dependencies are not available in the conda channels:

    • matchms
    • spectrum_utils
    • pyopenms
  2. The pytorch-gpu specification might be too restrictive:

    • Some users might prefer CPU-only installations
    • Consider using a metapackage or providing CPU/GPU variants

Consider these solutions:

  1. First ensure these packages are available in conda-forge:
    • Submit feedstock PRs for missing packages
    • Or use PyPI fallback for these specific packages
  2. For PyTorch, either:
    • Use pytorch as a metapackage
    • Or create CPU/GPU variants of this recipe

Let me verify the PyTorch package availability and naming conventions in conda-forge to provide a complete response.


Let me check for the main PyTorch package to complete the verification.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify package availability in conda-forge
for pkg in matchms spectrum_utils pyopenms; do
  echo "Checking $pkg in conda-forge..."
  gh repo view conda-forge/${pkg}-feedstock --json name,description,url 2>/dev/null || echo "Not found: $pkg"
done

Length of output: 771


Script:

#!/bin/bash
# Check pytorch packages in conda-forge
echo "Checking pytorch packages..."
gh search repos "pytorch-feedstock" --owner conda-forge --json name,description,url

# Check if pytorch-gpu exists as a separate package
echo -e "\nChecking pytorch-gpu specifically..."
gh repo view conda-forge/pytorch-gpu-feedstock --json name,description,url 2>/dev/null || echo "pytorch-gpu feedstock not found"

Length of output: 5521


Script:

#!/bin/bash
# Check main pytorch package
echo "Checking main pytorch package..."
gh repo view conda-forge/pytorch-feedstock --json name,description,url 2>/dev/null || echo "pytorch feedstock not found"

Length of output: 212


test:
imports:
- instanovo
commands:
- pip check
- python -c "import instanovo; print(instanovo.__version__)"
requires:
- pip

about:
home: https://github.com/instadeepai/instanovo
summary: De novo peptide sequencing with InstaNovo
license: Apache-2.0
license_file: LICENSE.md

extra:
recipe-maintainers:
- BioGeek
identifiers:
- doi:10.1101/2023.08.30.555055
Loading