Skip to content

Conversation

@Bento007
Copy link
Contributor

@Bento007 Bento007 commented Sep 12, 2025

  • This Pr fixes an issue where sorting the fragment was crashing when calling pigz for compression. This was only observed when sorting fragments larger than can fit into memory.

  • This PR refactors the ATAC-seq fragment processing functions to improve code organization, error handling, and maintainability. The changes consolidate duplicate pipeline logic and provide more robust subprocess management.

  • Refactored prepare_fragment() and deduplicate_fragment_rows() functions to use a shared pipeline infrastructure
    Introduced helper functions for command building, tool validation, and subprocess execution
    Improved error handling and resource cleanup for subprocess pipelines

  • This will require a 6.x release

@Bento007 Bento007 requested a review from Copilot September 12, 2025 22:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ATAC-seq fragment processing functions to improve code organization, error handling, and maintainability. The changes consolidate duplicate pipeline logic and provide more robust subprocess management.

  • Refactored prepare_fragment() and deduplicate_fragment_rows() functions to use a shared pipeline infrastructure
  • Introduced helper functions for command building, tool validation, and subprocess execution
  • Improved error handling and resource cleanup for subprocess pipelines

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

❌ Patch coverage is 95.57522% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.01%. Comparing base (8957d44) to head (5e9eeae).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1447      +/-   ##
==========================================
+ Coverage   88.50%   89.01%   +0.50%     
==========================================
  Files          23       23              
  Lines        2662     2703      +41     
==========================================
+ Hits         2356     2406      +50     
+ Misses        306      297       -9     
Components Coverage Δ
cellxgene_schema_cli 89.48% <95.57%> (+0.65%) ⬆️
migration_assistant 91.26% <ø> (ø)
schema_bump_dry_run_genes 79.74% <ø> (ø)
schema_bump_dry_run_ontologies 99.51% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Bento007 Bento007 requested a review from Copilot September 13, 2025 03:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

:return: Percentage of memory to allocate per sort thread.
"""
return max(sort_memory_percent // num_cores, 1) # ensure at least 1% memory per core
effective_memory_pct = min(sort_memory_percent, 50) if num_cores > 1 else sort_memory_percent
Copy link

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

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

The hardcoded 50% memory cap lacks explanation. This magic number should be documented or made configurable, as different environments may benefit from different memory limits.

Copilot uses AI. Check for mistakes.
@Bento007 Bento007 changed the title Tsmith/dedupe error 13 fix: dedupe error 13 Sep 13, 2025
@Bento007 Bento007 enabled auto-merge (squash) September 15, 2025 17:59
@Bento007 Bento007 merged commit a510c32 into main Sep 15, 2025
10 of 11 checks passed
@Bento007 Bento007 deleted the tsmith/dedupe-error-13 branch September 15, 2025 18:04
Bento007 added a commit that referenced this pull request Sep 26, 2025
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.

3 participants