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

ms2rescore fix version for DeepLC #34

Merged
merged 123 commits into from
Nov 22, 2024
Merged

ms2rescore fix version for DeepLC #34

merged 123 commits into from
Nov 22, 2024

Conversation

ypriverol
Copy link
Member

@ypriverol ypriverol commented Nov 21, 2024

Summary by CodeRabbit

  • New Features
    • Updated the project version to 0.0.14.
    • Added the deepLC package to dependencies.
    • Re-added the psm-utils package to dependencies.
  • Bug Fixes
    • Removed the psm-utils package from dependencies in some files.
    • Retained the ms2rescore package at version 3.0.3.
  • Improvements
    • Enhanced dependency management with updated package versions and restructuring.
    • Simplified the build workflow by removing the installation step and adding a new output folder specification.

@ypriverol ypriverol changed the title ms2rescore fix versions for DeepLC and ms2pip ms2rescore fix version for DeepLC Nov 21, 2024
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)
.github/workflows/conda-build.yml (1)

51-51: Add missing newline at end of file

The file is missing a newline character at the end, as reported by yamllint.

Add a newline at the end of the file to comply with text file standards.

🧰 Tools
🪛 yamllint (1.29.0-1)

[error] 51-51: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between dd36f48 and 4ecfad8.

📒 Files selected for processing (1)
  • .github/workflows/conda-build.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.29.0-1)
.github/workflows/conda-build.yml

[error] 51-51: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (1)
.github/workflows/conda-build.yml (1)

42-43: LGTM: Improved version command output formatting

Adding a newline after the version command output improves readability of CI logs.

.github/workflows/conda-build.yml 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: 0

🧹 Outside diff range and nitpick comments (1)
.github/workflows/conda-build.yml (1)

38-43: Enhance version verification step

The installation verification could be more robust by explicitly checking the installed version matches the expected version.

 mamba create --name quantmsutils quantms-utils==0.0.14 --use-local
 conda activate quantmsutils
 conda list
 quantmsutilsc --help
-quantmsutilsc --version
+version=$(quantmsutilsc --version)
+if [[ "$version" != *"0.0.14"* ]]; then
+  echo "Error: Unexpected version $version"
+  exit 1
+fi
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4ecfad8 and ee3c973.

📒 Files selected for processing (1)
  • .github/workflows/conda-build.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.29.0-1)
.github/workflows/conda-build.yml

[error] 51-51: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (2)
.github/workflows/conda-build.yml (2)

44-47: Previous review comment about error handling remains applicable

The suggested improvements from the previous review regarding error handling, checksum verification, and file cleanup are still valid and should be implemented.


48-51: Previous review comment about environment verification remains applicable

The suggested improvements from the previous review regarding environment verification and error handling are still valid and should be implemented.

Add newline at end of file

Add a newline character at the end of the file to comply with POSIX standards.

 conda activate quantmsutils
-        quantmsutilsc ms2rescore --psm_file TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01_comet.idXML --spectrum_path TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01.mzML --processes 2 --ms2pip_model HCD2021 --feature_generators 'ms2pip,deeplc' --id_decoy_pattern ^rev --test_fdr 0.05
+        quantmsutilsc ms2rescore --psm_file TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01_comet.idXML --spectrum_path TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01.mzML --processes 2 --ms2pip_model HCD2021 --feature_generators 'ms2pip,deeplc' --id_decoy_pattern ^rev --test_fdr 0.05
+
🧰 Tools
🪛 yamllint (1.29.0-1)

[error] 51-51: no new line character at the end of file

(new-line-at-end-of-file)

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)
.github/workflows/conda-build.yml (1)

38-44: Enhance package verification steps

While the basic verification is good, consider adding more thorough checks:

 mamba create --name quantmsutils python=3.11
 conda activate quantmsutils
 mamba install --use-local quantms-utils==0.0.14
 conda list
+# Verify all required executables are available
+which quantmsutilsc
+# Verify all key dependencies are installed correctly
+python -c "import ms2rescore; import deeplc; print('Dependencies OK')"
 quantmsutilsc --help
 quantmsutilsc --version
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ee3c973 and 4b988c7.

📒 Files selected for processing (1)
  • .github/workflows/conda-build.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.29.0-1)
.github/workflows/conda-build.yml

[error] 52-52: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (2)
.github/workflows/conda-build.yml (2)

45-48: Reference existing review comment

The previous review comment about adding error handling and file verification to the download step is still valid and should be addressed.


38-52: Verify workflow with DeepLC integration

Since this PR focuses on fixing versions for DeepLC, let's verify the workflow runs successfully with the new integration.

✅ Verification successful

Let me try a different approach to verify the DeepLC integration.


DeepLC integration is properly configured and consistent

The verification shows that:

  • DeepLC version 2.2.38 is consistently specified across all dependency files (pyproject.toml, environment.yml, requirements.txt, and recipe/meta.yaml)
  • DeepLC is properly integrated in the ms2rescore functionality (quantmsutils/rescoring/ms2rescore.py)
  • The workflow correctly sets up the conda environment with the required dependencies

The silent failure concern is addressed by the workflow's structure:

  • The conda environment setup includes explicit version checks (quantmsutilsc --version)
  • The ms2rescore command uses DeepLC as part of the default feature generators
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify DeepLC integration in the workflow

# Check if DeepLC is properly referenced in dependency files
echo "Checking DeepLC version consistency..."
rg -l "deeplc.*2\.2\.38" 

# Check for any existing workflow runs with DeepLC
echo "Checking previous workflow runs..."
gh run list --workflow conda-build.yml --json conclusion,headBranch | \
  jq '.[] | select(.headBranch=="ms2rescore-fix-version-for-deeplc")'

Length of output: 332


Script:

#!/bin/bash
# Check for DeepLC in conda/mamba environment setup
echo "Checking conda environment setup..."
rg -A 5 "mamba create" .github/workflows/conda-build.yml

# Check for DeepLC-related configurations and dependencies
echo "Checking DeepLC configurations..."
fd -e yml -e yaml | xargs rg "deeplc" -i

# Check for DeepLC usage in the codebase
echo "Checking DeepLC usage..."
rg "deeplc" -i

Length of output: 1375

🧰 Tools
🪛 yamllint (1.29.0-1)

[error] 52-52: no new line character at the end of file

(new-line-at-end-of-file)

.github/workflows/conda-build.yml 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: 0

🧹 Outside diff range and nitpick comments (1)
.github/workflows/conda-build.yml (1)

44-44: Add newline at end of file

The YAML file is missing a newline character at the end of the file, which is a common best practice and highlighted by the linter.

   quantmsutilsc ms2rescore --psm_file TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01_comet.idXML --spectrum_path TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01.mzML --processes 2 --ms2pip_model HCD2021 --feature_generators 'ms2pip,deeplc' --id_decoy_pattern ^rev --test_fdr 0.05
+
🧰 Tools
🪛 yamllint (1.29.0-1)

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4b988c7 and c968306.

📒 Files selected for processing (1)
  • .github/workflows/conda-build.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.29.0-1)
.github/workflows/conda-build.yml

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (3)
.github/workflows/conda-build.yml (3)

35-35: LGTM: Package installation looks good

The explicit version pinning ensures reproducible builds, and using mamba for installation is a good practice for better performance.


37-40: Previous review comment about error handling is still applicable

The existing review comment about adding error handling and file verification for the download step remains valid and unaddressed.


42-44: Previous review comment about test execution improvements is still applicable

The existing review comment about improving test execution with error handling and better formatting remains valid and unaddressed.

🧰 Tools
🪛 yamllint (1.29.0-1)

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

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

Reviewing files that changed from the base of the PR and between 2d4413b and 832b8f2.

📒 Files selected for processing (1)
  • .github/workflows/conda-build.yml (1 hunks)
🔇 Additional comments (2)
.github/workflows/conda-build.yml (2)

37-40: Previous review comment about error handling remains valid

The suggested improvements for error handling and file verification from the previous review have not been addressed.


41-43: Add DeepLC output validation

While the previous review comment about error handling remains valid, additional validation for DeepLC-specific outputs is needed since this PR specifically addresses DeepLC integration.

Add validation for DeepLC outputs:

 quantmsutilsc ms2rescore --psm_file TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01_comet.idXML --spectrum_path TMT_Erwinia_1uLSike_Top10HCD_isol2_45stepped_60min_01.mzML --processes 2 --ms2pip_model HCD2021 --feature_generators 'ms2pip,deeplc' --id_decoy_pattern ^rev --test_fdr 0.05
+
+# Verify DeepLC-specific outputs
+for file in *_rt_predictions.csv *_deeplc_features.csv; do
+  if [[ ! -s "$file" ]]; then
+    echo "Error: DeepLC output file $file is missing or empty"
+    exit 1
+  fi
+done
+
+# Validate DeepLC predictions format
+if ! head -n1 *_rt_predictions.csv | grep -q "^sequence,prediction"; then
+  echo "Error: DeepLC predictions file has unexpected format"
+  exit 1
+fi

.github/workflows/conda-build.yml 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

🧹 Outside diff range and nitpick comments (1)
.github/workflows/conda-build.yml (1)

34-34: Add newline at end of file

The file is missing a newline character at the end.

Add a newline after the last line:

         conda build recipe --suppress-variables --override-channels --channel conda-forge --channel bioconda --no-anaconda-upload --output-folder ./
+
🧰 Tools
🪛 yamllint (1.29.0-1)

[error] 34-34: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3b58ea9 and 09e83f1.

📒 Files selected for processing (1)
  • .github/workflows/conda-build.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.29.0-1)
.github/workflows/conda-build.yml

[error] 34-34: no new line character at the end of file

(new-line-at-end-of-file)

.github/workflows/conda-build.yml Show resolved Hide resolved
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