Skip to content

Trans eQTL runner - #263

Open
htanudisastro wants to merge 6 commits into
mainfrom
trans_pilot
Open

Trans eQTL runner#263
htanudisastro wants to merge 6 commits into
mainfrom
trans_pilot

Conversation

@htanudisastro

Copy link
Copy Markdown
Contributor

Remove --regions so that it runs all variants in the vcf.

@htanudisastro
htanudisastro requested a review from Copilot May 9, 2025 06:02

Copilot AI left a comment

Copy link
Copy Markdown

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 updates the trans eQTL runner by removing the --regions option so that all variants in the VCF are processed. Additionally, it introduces a new configuration file and updates the runner script to align with that change.

  • Added new configuration settings in trans_associatr_runner.toml.
  • Updated the runner script to no longer pass --regions to associaTR.

Reviewed Changes

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

File Description
str/trans-eqtl/trans_associatr_runner.toml Added new configuration options without the --regions parameter.
str/trans-eqtl/trans_associatr_runner.py Updated the job command and concurrency handling logic to support processing all variants.

Comment thread str/trans-eqtl/trans_associatr_runner.py
Comment thread str/trans-eqtl/trans_associatr_runner.py Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@htanudisastro
htanudisastro requested a review from MattWellie May 9, 2025 06:04
@htanudisastro

Copy link
Copy Markdown
Contributor Author

Is there a way to bypass the Ubuntu image check or do I have to update the image?

Run associaTR processing pipeline
"""
b = get_batch(name='Run associatr')
init_batch()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed here? init_batch starts a hail query runtime, but you only need a batch

Comment on lines +51 to +56
vcf_file_path = get_config()['associatr']['vcf_file_path']
variant_vcf = b.read_input_group(
**dict(
base=vcf_file_path,
),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this can be moved outside the loops, the VCF isn't specific to celltypes or chromosomes

import hailtop.batch as hb

from cpg_utils import to_path
from cpg_utils.config import get_config

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you might want to use config_retrieve, the syntax is a little cleaner, and has built-in informative error messaging if you try and access keys which don't exist, e.g.

get_config()['associatr']['celltypes']
vs.
config_retrieve(['associatr', 'celltypes'])

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