Files that changed from the base of the PR and between 19a2ab7 and 72c8e64.

• .github/workflows/main.yml (2 hunks)
• snakemake/deployment/conda.py (7 hunks)
• snakemake/settings/types.py (2 hunks)
• test-environment.yml (0 hunks)
• tests/common.py (1 hunks)

• test-environment.yml

.github/workflows/main.yml (2)

147-150: LGTM! Consistent with Linux job changes.

The changes in the Windows job mirror those made in the Linux job, which is good for consistency across platforms. This update successfully aligns the Windows environment setup with the new conda-based approach.

As mentioned in the previous comment, consider pinning the miniforge-version to a specific version for reproducibility.

---

Line range hint 1-164: Inconsistency in formatting job and clarification needed on caching.

1. The formatting job still uses mamba-org/setup-micromamba@v1. For consistency with the other jobs and alignment with the PR objectives, consider updating this to use conda-incubator/setup-miniconda@v3 as well.

2. The cache-environment option has been removed from both the formatting and testing jobs. While this change is not directly related to the PR objectives, it might affect the CI performance. Could you provide some context on why this change was made?

To verify the inconsistency and the removal of caching, you can run the following script:

This script will help identify any remaining instances of mamba usage and confirm the removal of the cache-environment option.

snakemake/settings/types.py (3)

252-252: Update default conda_frontend to 'conda'.

The default value of conda_frontend has been correctly updated to 'conda' to reflect the deprecation of mamba support.

---

262-263: Import logger for warning messages.

The addition of logger import is necessary for issuing deprecation warnings in __post_init__.

---

268-276: Add deprecation warning and reset for alternative conda frontends.

The code appropriately warns users about the deprecation of alternative conda frontends and resets self.conda_frontend to 'conda' to ensure consistent behavior.

snakemake/deployment/conda.py (6)

92-92: Removal of frontend parameter is correctly reflected

The instantiation of Conda in the conda property now omits the frontend parameter, consistent with the updated Conda class definition where frontend is no longer required.

---

532-532: Updated subcommand to use conda exclusively

The subcommand is now set to ["conda"], reflecting the deprecation of mamba support and ensuring that only conda is used for environment creation.

---

653-653: Removed frontend parameter from Conda.__new__ method

The __new__ method signature no longer includes the frontend parameter, aligning with the decision to simplify the Conda class by removing support for multiple frontends.

---

662-662: Removed frontend parameter from Conda.__init__ method

The __init__ method now excludes the frontend parameter, consistent with the refactoring to eliminate mamba support and streamline the initialization process.

---

698-725: Simplified command availability check to focus on conda

The _check method now sets frontend = "conda" and removes conditional logic related to mamba. This change aligns with removing mamba support and simplifies the command availability check.

🧰 Tools

🪛 Ruff

710-710: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)

---

763-769: Updated minimum required conda version to 24.9.1

The version check now requires conda version 24.9.1 or later, which aligns with the PR objective to rely on the latest conda features. This update ensures compatibility with the required functionalities introduced in newer conda versions.

Files that changed from the base of the PR and between 72c8e64 and c55cf8b.

• snakemake/deployment/conda.py (8 hunks)

snakemake/deployment/conda.py (1)

701-740: Duplicate Comment: Address unused variable e and enhance exception chaining

The variable e is assigned in the except clause at line 713 but not utilized when raising the new exception at line 740. To preserve the original traceback and provide more context, it's advisable to use exception chaining with raise ... from e.

This issue has been previously noted and the past review comment is still valid and applicable.

🧰 Tools

🪛 Ruff

713-713: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)

---

740-740: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

Files that changed from the base of the PR and between c55cf8b and e0340ae.

• tests/test_containerized/expected-results/test.out is excluded by !**/*.out

• snakemake/cli.py (1 hunks)
• snakemake/deployment/conda.py (9 hunks)
• snakemake/deployment/containerize.py (2 hunks)
• tests/test_containerized/Dockerfile (1 hunks)
• tests/test_containerized/Snakefile (1 hunks)
• tests/test_containerized/env.yaml (1 hunks)

tests/test_containerized/env.yaml (3)

4-4: Excellent addition of the 'nodefaults' channel.

Adding the 'nodefaults' channel is a good practice for ensuring reproducibility. It prevents conda from automatically including default channels, which can sometimes lead to inconsistent environments across different systems. This change aligns well with best practices for conda environment management.

---

6-6: Approved: BCFtools version update to 1.21.

Updating BCFtools from version 1.10 to 1.21 is a significant upgrade that likely brings important improvements and bug fixes. This update aligns with the PR's objective of using more recent software versions.

To ensure this update doesn't introduce unexpected changes to your workflows, please verify:

1. Compatibility with existing scripts and pipelines that use BCFtools.
2. Any changes in command syntax or output format that might affect downstream processes.

Consider running the following command to check for any major changes in the BCFtools usage:

This script will highlight any differences in the command-line interface between the two versions, which could indicate potential breaking changes.

---

1-6: Verify completeness of dependencies for containerized tests.

The environment file appears to be minimal, containing only one dependency (bcftools). While this might be intentional for a specific test case, it's worth verifying if all necessary dependencies for the containerized tests are included.

To ensure all required dependencies are present, consider running the following command to check for any import errors or missing dependencies in the test files:

This script will help identify any potential missing dependencies by checking for import errors in the test files. If any missing dependencies are found, consider adding them to the env.yaml file.

tests/test_containerized/Snakefile (1)

1-1: LGTM! Verify the new Docker image.

The update to the containerized directive looks good. The Docker image has been updated from version 1.0 to 1.2, and the repository has changed from "quay.io/snakemake" to "snakemake".

To ensure the workflow continues to function as expected, please verify:

1. The new image "snakemake/containerize-testimage:1.2" exists and is accessible.
2. The new image is compatible with the current workflow and contains all necessary dependencies.

You can use the following command to check if the image exists:

If the image doesn't exist or there are any issues, please update the directive accordingly.

tests/test_containerized/Dockerfile (5)

1-1: Approved: Base image change aligns with PR objectives

The change from condaforge/mambaforge:latest to condaforge/miniforge3:latest as the base image is consistent with the PR's goal of deprecating mamba support and relying on conda. This modification ensures that the testing environment uses a minimal Conda-based image, which is appropriate for the new approach.

---

15-15: Verify compatibility with updated bcftools version

The update of bcftools from version 1.10 to 1.21 is a significant version jump that likely includes new features and bug fixes. This update is generally positive for keeping dependencies up-to-date.

To ensure compatibility, please run the following script to check for any breaking changes or deprecations between these versions:

#!/bin/bash
# Check for potential breaking changes in bcftools update

echo "Checking for potential breaking changes in bcftools update from 1.10 to 1.21"

# Search for bcftools usage in the codebase
echo "Files using bcftools:"
rg --type python "bcftools" -l

echo "Please manually review the bcftools usage in these files and compare with the changelog of bcftools 1.21 to ensure compatibility."

# Optionally, you can add a step to fetch and display the changelog between these versions
# This would require additional setup to access the bcftools repository or release notes

---

9-9: Ensure consistency of the conda environment prefix

The update to the conda environment prefix is appropriate and likely reflects changes in the overall conda setup process. This modification is consistent with the PR's objectives.

To ensure consistency, please run the following script to check if the prefix is used consistently throughout the Dockerfile:

#!/bin/bash
# Check for consistency of the conda environment prefix in the Dockerfile

prefix=$(grep "prefix:" tests/test_containerized/Dockerfile | awk '{print $3}')
echo "Conda environment prefix: $prefix"

# Check if the prefix is used consistently
if grep -q "$prefix" tests/test_containerized/Dockerfile; then
    echo "Prefix is used consistently in the Dockerfile."
else
    echo "Warning: Prefix might not be used consistently in the Dockerfile."
    echo "Please manually verify the usage of the prefix."
fi

---

21-22: Approved: Conda environment creation aligns with PR objectives

The modification of the environment creation commands to use 'conda' instead of 'mamba' directly implements the PR's goal of deprecating mamba support. The adjustment of paths for environment creation and the environment.yaml file is consistent with the earlier prefix changes.

To ensure the conda environment is created correctly, please run the following script:

#!/bin/bash
# Verify conda environment creation command

# Extract the conda env creation command from the Dockerfile
conda_command=$(grep "RUN conda env create" tests/test_containerized/Dockerfile)

echo "Conda environment creation command:"
echo "$conda_command"

# Check if the command uses 'conda' and not 'mamba'
if [[ $conda_command == *"conda env create"* ]] && [[ $conda_command != *"mamba"* ]]; then
    echo "Verification successful: The command uses 'conda' for environment creation."
else
    echo "Verification failed: Please check the conda environment creation command."
fi

# Verify the paths used in the command
prefix=$(echo "$conda_command" | grep -oP "(?<=--prefix )[^ ]+")
yaml_file=$(echo "$conda_command" | grep -oP "(?<=--file )[^ ]+")

echo "Verifying paths:"
echo "Prefix: $prefix"
echo "YAML file: $yaml_file"

# Check if the paths are consistent with earlier changes
if [[ $prefix == "/conda-envs/2a132eb0d84044eaf02032bd9f9851fd" ]] && 
   [[ $yaml_file == "/conda-envs/2a132eb0d84044eaf02032bd9f9851fd/environment.yaml" ]]; then
    echo "Path verification successful: The paths are consistent with earlier changes."
else
    echo "Path verification failed: Please check the consistency of paths in the conda command."
fi

---

3-3: Verify the conda environment hash

The update to the conda environment hash label is appropriate, as it likely reflects changes in the conda environment definition. This update is crucial for maintaining proper caching and reproducibility of the conda environment.

To ensure the hash is correct, please run the following script:

snakemake/deployment/containerize.py (1)

74-79: Approve the step renumbering and cleanup command changes.

The changes in step numbering and the switch from mamba clean to conda clean are consistent with the overall objective of deprecating mamba support. The -y flag is maintained, ensuring non-interactive execution during the containerization process.

snakemake/cli.py (2)

Line range hint 1-1629: Summary of changes and recommendations

The primary change in this file is the update to the --conda-frontend argument, switching the default from "mamba" to "conda". This change aligns with the PR objective to rely on conda >24.7.1 and deprecate mamba support.

Key points and recommendations:

1. Update the help text for --conda-frontend to reflect the new default and explain the rationale.
2. Consider broader implications including documentation updates, backward compatibility, performance monitoring, and user communication.
3. Implement a deprecation warning for explicit mamba usage.
4. Add performance benchmarking to validate the change.
5. Verify and update any other parts of the codebase that might assume mamba as the default.

These changes and considerations will ensure a smooth transition from mamba to conda as the default frontend while maintaining clear communication with users about the change and its implications.

---

Line range hint 1-1629: Consider broader impact of conda frontend change

While the change to the --conda-frontend default is localized to a single argument, it may have broader implications for the Snakemake workflow and user experience.

1. Documentation: Ensure that all relevant documentation (both in-code and external) is updated to reflect this change in default behavior.

2. Backward Compatibility: Consider adding a deprecation warning if users explicitly set --conda-frontend mamba, informing them about the change and future plans.

3. Performance Monitoring: As the change is motivated by conda's improved performance with the libmamba solver, it might be beneficial to add performance monitoring or benchmarking to ensure the change meets expectations.

4. User Communication: Plan for how this change will be communicated to users, possibly through release notes or a migration guide.

To facilitate these considerations, you might want to:

1. Add a deprecation warning in the argument parsing logic when mamba is explicitly selected.
2. Implement a simple benchmarking function to compare conda and mamba performance in your test suite.
3. Update the user guide and any examples that might reference mamba as the recommended or default option.

To ensure no other parts of the code assume mamba as the default, we can search for such occurrences:

✅ Verification successful

No further issues found regarding the --conda-frontend default to mamba

After comprehensive verification, all instances of mamba usage within the codebase are appropriately handled. The change to set mamba as the default conda frontend in snakemake/cli.py is consistent with existing documentation, Docker configurations, and help descriptions. This adjustment aligns with the goal of leveraging libmamba for improved performance without introducing any inconsistencies or overlooked references.

Action Items:

• Ensure that all user-facing documentation has been reviewed and accurately reflects the change to mamba as the default frontend.
• Monitor user feedback post-release to identify any unforeseen issues related to this change.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for 'mamba' occurrences that might need updating
rg 'mamba' --type python

Length of output: 55

---

Script:

#!/bin/bash
# Corrected command to search for 'mamba' in Python files
rg 'mamba' --glob "*.py"

Length of output: 417

---

Script:

#!/bin/bash
# Search for 'mamba' across all files to identify any additional usages
rg 'mamba'

Length of output: 3283

snakemake/deployment/conda.py (3)

Line range hint 536-541: Confirm absence of --yes flag to ensure non-interactive environment creation

At lines 536-541, when filetype is 'yaml', the yes_flag list is set to empty:

yes_flag = ["--yes"]
if filetype == "yaml":
    subcommand.append("env")
    yes_flag = []

This means the --yes flag is omitted from the Conda command when creating environments from YAML files. This could cause Conda to prompt for confirmation, potentially interrupting automated workflows.

Please confirm whether omitting the --yes flag is intentional and that environment creation proceeds without user interaction. If non-interactive execution is desired, consider retaining the --yes flag:

if filetype == "yaml":
    subcommand.append("env")
-   yes_flag = []

---

758-770: Ensure MIN_CONDA_VER aligns with required minimum version

At line 765, the code checks if the Conda version is less than MIN_CONDA_VER (which is "24.7.1" as defined at line 46). According to the PR objectives, the minimum required Conda version should be greater than 24.9.1. Please update MIN_CONDA_VER to reflect the correct minimum version to ensure consistency and prevent version mismatch errors.

(This issue has been previously addressed in past review comments.)

---

714-741: Implement exception chaining for clearer error context

In the exception handling block starting at line 714, the variable e is assigned but not utilized when raising the new exception. To preserve the original exception context and provide more detailed traceback information, consider using exception chaining with raise ... from e.

(This issue has been previously addressed in past review comments.)

🧰 Tools

🪛 Ruff

714-714: Local variable e is assigned to but never used

Remove assignment to unused variable e

(F841)

---

741-741: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

Files that changed from the base of the PR and between e0340ae and 9b05188.

• tests/test_conda_function/Snakefile (1 hunks)
• tests/test_conda_named/Snakefile (1 hunks)

tests/test_conda_named/Snakefile (1)

Line range hint 1-31: Overall, the changes look good and align with the PR objectives.

The switch from mamba to conda in the environment creation command is implemented correctly. The rest of the file remains consistent with this change. The suggested improvements (checking conda version and verifying conda environment usage) would further enhance the test's effectiveness in ensuring the correct behavior of the updated conda-based workflow.

tests/test_conda_function/Snakefile (1)

Line range hint 20-37: Verify ripgrep versions in test cases

The test cases use ripgrep versions 12.1.1 and 13.0.0. While these haven't changed, it's worth verifying if these versions are still appropriate for testing, especially considering the recent changes in the conda environment setup.

To check the latest available versions of ripgrep, you can run:

This will show the 5 most recent versions of ripgrep available in the conda-forge channel. If there are significantly newer versions available, consider updating the test cases to include a more recent version.

Files that changed from the base of the PR and between 9b05188 and 29f466c.

• .github/workflows/main.yml (2 hunks)
• setup.cfg (1 hunks)
• snakemake/deployment/conda.py (12 hunks)
• test-environment.yml (1 hunks)
• tests/tests.py (2 hunks)

• test-environment.yml

setup.cfg (1)

54-54: Approve the dependency version update.

The update to the snakemake-interface-executor-plugins dependency version (from >=9.2.0,<10.0 to >=9.3.2,<10.0) is appropriate. This change ensures the project uses a more recent version of the executor plugins interface while maintaining compatibility with versions below 10.0.

To ensure this change doesn't introduce any unexpected issues, please verify:

1. The compatibility of this updated dependency version with the rest of the project.
2. That the changelog or release notes for snakemake-interface-executor-plugins between versions 9.2.0 and 9.3.2 don't contain any breaking changes or deprecations that might affect Snakemake.

You can use the following script to check for any potential issues:

✅ Verification successful

Dependency version update verified.

The update to the snakemake-interface-executor-plugins dependency version (from >=9.2.0,<10.0 to >=9.3.2,<10.0) has been verified. No usages or explicit version checks were found in the codebase, ensuring that the update maintains compatibility without introducing issues.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential compatibility issues with the updated dependency

# Test: Search for usage of snakemake-interface-executor-plugins in the codebase
echo "Searching for usage of snakemake-interface-executor-plugins:"
rg --type python "from snakemake_interface_executor_plugins import" -A 5

# Test: Check if there are any explicit version checks for this dependency
echo "Checking for explicit version checks:"
rg --type python "snakemake_interface_executor_plugins.__version__"

# Note: Review the output manually to ensure there are no compatibility issues

Length of output: 420

---

Script:

#!/bin/bash
# Description: Identify Python files and search for usage of snakemake-interface-executor-plugins

# Step 1: Find all Python files
echo "Finding all Python files in the repository:"
fd --extension py > python_files.txt
cat python_files.txt

# Step 2: Search for usage of snakemake-interface-executor-plugins in Python files
echo "Searching for usage of snakemake-interface-executor-plugins in Python files:"
xargs -a python_files.txt -I {} rg "from snakemake_interface_executor_plugins import" {} -A 5

# Step 3: Check for explicit version checks of snakemake-interface-executor-plugins
echo "Checking for explicit version checks of snakemake-interface_executor_plugins:"
xargs -a python_files.txt -I {} rg "snakemake_interface_executor_plugins\.\_\_version\_\_" {} 

# Cleanup
rm python_files.txt

Length of output: 24569

---

Script:

#!/bin/bash
# Description: Identify Python files and search for usage of snakemake-interface-executor-plugins

# Step 1: Find all Python files
echo "Finding all Python files in the repository:"
fd --extension py > python_files.txt
cat python_files.txt

# Step 2: Search for usage of snakemake-interface-executor-plugins in Python files
echo "Searching for usage of snakemake-interface-executor-plugins in Python files:"
xargs -a python_files.txt -I {} rg "from snakemake_interface_executor_plugins import" {} -A 5

# Step 3: Check for explicit version checks of snakemake-interface-executor-plugins
echo "Checking for explicit version checks of snakemake_interface_executor_plugins:"
xargs -a python_files.txt -I {} rg "snakemake_interface_executor_plugins\.__version__" {}

# Cleanup
rm python_files.txt

Length of output: 4341

tests/tests.py (1)

499-499: Update to use new function for checking environment setup.

The code has been updated to use the get_env_setup_done_flag_file function to check for the existence of the environment setup done flag file. This change improves the robustness of the test by using a centralized function to determine the correct path for the flag file.

To ensure that the get_env_setup_done_flag_file function is correctly implemented and used, we can run the following verification: