-
Notifications
You must be signed in to change notification settings - Fork 2
Contamination removal, fastq preprocessing #20
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
Changes from all commits
910ffa0
dd9547a
ee7d65d
fc11717
07404fb
45917b0
8a7ebf4
df9fb3a
e42e4e8
fde16bb
c6e827d
4828717
a7dada2
0c70a0e
317e617
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| name: Conformity Tests | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main, master] | ||
| pull_request: | ||
| branches: [main, master] | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| conformity: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.11" | ||
|
|
||
| - name: Install test dependencies | ||
| run: pip install -r tests/requirements.txt | ||
|
|
||
| - name: Run conformity tests | ||
| run: pytest tests/ -v -m "not network" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| [pytest] | ||
| markers = | ||
| network: marks tests requiring network access (deselect with '-m "not network"') |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -204,8 +204,7 @@ def create_sequence_dictionary(fasta_file): | |
| """ | ||
| Create index and sequence dictionary for a FASTA file using samtools | ||
| """ | ||
| dict_file = f"{''.join(fasta_file.split('.')[:-1])}.dict" | ||
| fasta_file.replace(".fa", ".dict").replace(".fasta", ".dict") | ||
| dict_file = f"{''.join(fasta_file.split('.')[:-1])}.dict" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a suffix-safe path builder for the Line [207] removes all dots from the full path ( Suggested fix- dict_file = f"{''.join(fasta_file.split('.')[:-1])}.dict"
+ dict_file = str(Path(fasta_file).with_suffix(".dict"))🤖 Prompt for AI Agents |
||
| index_file = fasta_file + ".fai" | ||
| for file in [dict_file, index_file]: | ||
| if os.path.isfile(file): | ||
|
|
@@ -229,6 +228,32 @@ def download_deepvariant_model_files(urls: list, outfolder: str): | |
| return destpath | ||
|
|
||
|
|
||
| def download_sortmerna_db(url, keep_file, outfolder): | ||
| """ | ||
| Download SortMeRNA database, extract only the needed file, and cleanup. | ||
| """ | ||
| dest_file = os.path.join(outfolder, keep_file) | ||
| if os.path.isfile(dest_file): | ||
| logging.info(f"{keep_file} already exists. Skipping.") | ||
| return dest_file | ||
|
|
||
| tar_filename = url.split("/")[-1] | ||
| tar_path = os.path.join(outfolder, tar_filename) | ||
|
|
||
| logging.info(f"Downloading SortMeRNA database from {url}") | ||
| if not os.path.isfile(tar_path): | ||
| run_command(["wget", "-c", url, "-P", outfolder]) | ||
|
|
||
| logging.info(f"Extracting {keep_file} from archive") | ||
| run_command(["tar", "-xzf", tar_path, "-C", outfolder, f"--wildcards", f"*/{keep_file}", "--strip-components=1"]) | ||
|
|
||
|
Comment on lines
+247
to
+249
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Confirm the offending f-string
rg -n 'f"--wildcards"' setup/download_res.pyRepository: ctglab/ENEO Length of output: 176 🏁 Script executed: #!/bin/bash
# Let's also check the actual line to see the full context
if [ -f setup/download_res.py ]; then
sed -n '248,250p' setup/download_res.py
fiRepository: ctglab/ENEO Length of output: 230 Remove the unnecessary f-string to satisfy Ruff F541.
🧹 Minimal fix- run_command(["tar", "-xzf", tar_path, "-C", outfolder, f"--wildcards", f"*/{keep_file}", "--strip-components=1"])
+ run_command(["tar", "-xzf", tar_path, "-C", outfolder, "--wildcards", f"*/{keep_file}", "--strip-components=1"])🧰 Tools🪛 Ruff (0.15.2)[error] 249-249: f-string without any placeholders Remove extraneous (F541) 🤖 Prompt for AI Agents |
||
| logging.info("Cleaning up archive") | ||
| if os.path.isfile(tar_path): | ||
| os.remove(tar_path) | ||
|
|
||
| return dest_file | ||
|
Comment on lines
+231
to
+254
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honor The new SortMeRNA downloader ignores dry-run and may update config even if extraction didn’t actually produce 🛠️ Suggested fix-def download_sortmerna_db(url, keep_file, outfolder):
+def download_sortmerna_db(url, keep_file, outfolder, dry=False):
@@
- logging.info(f"Downloading SortMeRNA database from {url}")
- if not os.path.isfile(tar_path):
- run_command(["wget", "-c", url, "-P", outfolder])
+ logging.info(f"Downloading SortMeRNA database from {url}")
+ if dry:
+ logging.info("Dry-run enabled; skipping download/extraction.")
+ return dest_file
+ if not os.path.isfile(tar_path):
+ run_command(["wget", "-c", url, "-P", outfolder])
@@
- run_command(["tar", "-xzf", tar_path, "-C", outfolder, f"--wildcards", f"*/{keep_file}", "--strip-components=1"])
+ run_command(["tar", "-xzf", tar_path, "-C", outfolder, "--wildcards", f"*/{keep_file}", "--strip-components=1"])
+ if not os.path.isfile(dest_file):
+ raise FileNotFoundError(f"Expected {dest_file} after extraction")
@@
- elif ftype == "sortmerna":
- path = download_sortmerna_db(res_entry['url'], res_entry['keep_file'], outfolder)
+ elif ftype == "sortmerna":
+ path = download_sortmerna_db(res_entry['url'], res_entry['keep_file'], outfolder, args.dry_run)Also applies to: 314-315 🧰 Tools🪛 Ruff (0.15.2)[error] 249-249: f-string without any placeholders Remove extraneous (F541) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| def convert_REDI(bed_url, bed_output, drop_intermediate=True): | ||
| if os.path.isfile(bed_output): | ||
| logging.info(f"{bed_output} already exists.") | ||
|
|
@@ -261,7 +286,7 @@ def main(args): | |
| if name not in resources and not os.path.isfile(existing_path): | ||
| logging.error(f"{name} missing in resources and not in repo.") | ||
| continue | ||
| if os.path.isfile(existing_path): | ||
| if os.path.isfile(existing_path) or os.path.isdir(existing_path): | ||
| logging.info(f"{name} already exists. Skipping.") | ||
| continue | ||
| res_entry = resources.get(name) | ||
|
|
@@ -285,6 +310,8 @@ def main(args): | |
| path = decompress_file(download_resource(res_entry, outfolder, args.dry_run)) | ||
| elif ftype == "model": | ||
| path = download_deepvariant_model_files(res_entry['url'], outfolder) | ||
| elif ftype == "sortmerna": | ||
| path = download_sortmerna_db(res_entry['url'], res_entry['keep_file'], outfolder) | ||
| else: | ||
| logging.warning(f"Unknown filetype for {name} as its {ftype}. Skipping.") | ||
| continue | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import json | ||
| import pathlib | ||
|
|
||
| import pytest | ||
| import yaml | ||
|
|
||
| ROOT = pathlib.Path(__file__).parent.parent | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def root(): | ||
| return ROOT | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def resources_json(): | ||
| return json.loads((ROOT / "setup" / "resources.json").read_text()) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def config(): | ||
| return yaml.safe_load((ROOT / "config" / "config_main.yaml").read_text()) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def rule_files(): | ||
| return list((ROOT / "workflow" / "rules").glob("*.smk")) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| pytest>=7.0 | ||
| pyyaml>=6.0 | ||
| requests>=2.28 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| """ | ||
| Validate config/config_main.yaml structure and cross-references with resources.json. | ||
| """ | ||
| import pathlib | ||
|
|
||
| import pytest | ||
| import yaml | ||
|
|
||
| ROOT = pathlib.Path(__file__).parent.parent | ||
|
|
||
| REQUIRED_TOP_LEVEL_KEYS = { | ||
| "OUTPUT_FOLDER", "TEMP_DIR", "datadirs", "params", "resources", "execution_mode" | ||
| } | ||
| VALID_EXECUTION_MODES = {"full", "CI"} | ||
| REQUIRED_PARAM_SECTIONS = { | ||
| "BQSR", "deepvariant", "fastp", "gatk", "MarkDuplicates", "pMHC", | ||
| "STAR", "SplitNCigarReads", "salmon", "samtools", "strelka2", "t1k", "vcfanno", "vep", | ||
| } | ||
|
|
||
|
|
||
| def test_config_parses(): | ||
| data = yaml.safe_load((ROOT / "config" / "config_main.yaml").read_text()) | ||
| assert isinstance(data, dict) and len(data) > 0 | ||
|
|
||
|
|
||
| def test_required_top_level_keys(config): | ||
| missing = REQUIRED_TOP_LEVEL_KEYS - set(config.keys()) | ||
| assert not missing, f"Missing top-level keys: {missing}" | ||
|
|
||
|
|
||
| def test_execution_mode_is_valid(config): | ||
| mode = config.get("execution_mode") | ||
| assert mode in VALID_EXECUTION_MODES, ( | ||
| f"execution_mode '{mode}' is not one of {VALID_EXECUTION_MODES}" | ||
| ) | ||
|
|
||
|
|
||
| def test_params_sections_present(config): | ||
| params = config.get("params", {}) | ||
| missing = REQUIRED_PARAM_SECTIONS - set(params.keys()) | ||
| assert not missing, f"Missing params sections: {missing}" | ||
|
|
||
|
|
||
| def test_all_downloadable_resources_in_config(config, resources_json): | ||
| config_resources = set(config.get("resources", {}).keys()) | ||
| json_resources = set(resources_json.keys()) | ||
| missing = json_resources - config_resources | ||
| assert not missing, ( | ||
| f"Resources defined in resources.json but absent from config.resources: {missing}" | ||
| ) | ||
|
|
||
|
|
||
| def test_datadirs_has_logs_section(config): | ||
| assert "logs" in config.get("datadirs", {}), "datadirs is missing 'logs' section" | ||
|
|
||
|
|
||
| def test_in_repo_resources_exist(config): | ||
| """Resources whose paths start with 'workflow/' must exist on disk.""" | ||
| resources = config.get("resources", {}) | ||
| missing = [] | ||
| for name, path in resources.items(): | ||
| if isinstance(path, str) and path.startswith("workflow/"): | ||
| full_path = ROOT / path | ||
| if not full_path.exists(): | ||
| missing.append(f"{name}: {path}") | ||
| assert not missing, "In-repo resources missing from disk:\n" + "\n".join(missing) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| """ | ||
| Validate setup/resources.json structure and completeness. | ||
| """ | ||
| import json | ||
| import pathlib | ||
|
|
||
| import pytest | ||
|
|
||
| ROOT = pathlib.Path(__file__).parent.parent | ||
| RESOURCES_PATH = ROOT / "setup" / "resources.json" | ||
|
|
||
| VALID_FILETYPES = {"vcf", "fasta", "gtf", "table", "archive", "model", "sortmerna"} | ||
|
|
||
|
|
||
| def test_resources_json_parses(): | ||
| data = json.loads(RESOURCES_PATH.read_text()) | ||
| assert isinstance(data, dict) and len(data) > 0 | ||
|
|
||
|
|
||
| def test_each_entry_has_required_fields(resources_json): | ||
| for name, entry in resources_json.items(): | ||
| assert "filetype" in entry, f"{name}: missing 'filetype'" | ||
| assert "url" in entry, f"{name}: missing 'url'" | ||
|
|
||
|
|
||
| def test_filetypes_are_known(resources_json): | ||
| for name, entry in resources_json.items(): | ||
| ft = entry["filetype"].lower() | ||
| assert ft in VALID_FILETYPES, f"{name}: unknown filetype '{ft}'" | ||
|
|
||
|
|
||
| def test_urls_are_nonempty_strings_or_lists(resources_json): | ||
| for name, entry in resources_json.items(): | ||
| url = entry["url"] | ||
| if isinstance(url, list): | ||
| assert len(url) > 0, f"{name}: url list is empty" | ||
| for u in url: | ||
| assert isinstance(u, str) and u.strip(), ( | ||
| f"{name}: url list contains empty or non-string entry" | ||
| ) | ||
| else: | ||
| assert isinstance(url, str) and url.strip(), ( | ||
| f"{name}: url is empty or not a string" | ||
| ) | ||
|
|
||
|
|
||
| def test_sortmerna_entries_have_keep_file(resources_json): | ||
| for name, entry in resources_json.items(): | ||
| if entry["filetype"].lower() == "sortmerna": | ||
| assert "keep_file" in entry, f"{name}: sortmerna entry missing 'keep_file'" | ||
| assert entry["keep_file"].strip(), f"{name}: keep_file is empty" | ||
|
|
||
|
|
||
| def test_model_entries_have_list_of_urls(resources_json): | ||
| for name, entry in resources_json.items(): | ||
| if entry["filetype"].lower() == "model": | ||
| assert isinstance(entry["url"], list), ( | ||
| f"{name}: model filetype should have a list of urls" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: ctglab/ENEO
Length of output: 345
🏁 Script executed:
Repository: ctglab/ENEO
Length of output: 708
🌐 Web query:
GitHub Actions security best practices pinning commit SHA vs tags💡 Result:
Pinning GitHub Actions to a full-length commit SHA is a critical security best practice, as it is the only way to ensure that an action is used as an immutable release [1][2]. Why Commit SHA Pinning is Recommended: - Immutability: Git tags and branches are mutable references that can be deleted, moved, or recreated by a repository owner or an attacker to point to different code [3][4][5]. Pinning to a full-length commit SHA guarantees that your workflow executes the exact code you expect every time, preventing "retargeting" attacks where malicious code is silently injected into a previously trusted version [3][6][5]. - Security Policy: GitHub now supports organization-level policies that can mandate full commit SHA pinning [1][7]. When enabled, any workflow referencing an action by a tag or branch will fail, forcing compliance across all repositories in an organization [3][6][7]. - Protection Against Compromise: If an action repository is compromised, pinning to a SHA ensures your pipeline continues to run the last known-good, reviewed code, rather than automatically adopting the attacker's latest malicious commit [4][6]. Best Practices for Implementation: - Use Inline Comments: When pinning to a SHA, include the original version tag as an inline comment (e.g., uses: actions/checkout@3df4ab1 # v4.0.0) [4][6][8]. This maintains human readability and allows automated dependency management tools to correlate the pin with semantic versions [4][9]. - Automate Updates: Manually maintaining SHAs is inefficient. Tools like Renovate (using presets such as helpers:pinGitHubActionDigestsToSemver) or Dependabot can be configured to manage these pins automatically, opening pull requests to update SHAs as new releases are published [4][6][8][10]. - Audit Existing Workflows: Use static analysis tools like CodeQL or Zizmor to identify unpinned actions in your repositories and audit them for migration to SHA pins [6][2]. While pinning to a tag is more convenient, it should only be done if you fully trust the creator and are aware of the risk that the tag's underlying code can change [1]. In professional or secure environments, moving away from tag-based references to SHA-pinned references is the standard for mitigating supply chain risks [7][5].
Citations:
Pin third-party actions to full commit SHAs and disable credential persistence.
Using floating tags (
@v4,@v5) exposes the workflow to upstream tag drift and retargeting attacks where malicious code can be silently injected into a previously trusted version. Full commit SHA pinning is the only way to ensure the exact code is executed every time. Additionally, this job does not need persisted checkout credentials.🔧 Suggested hardening patch
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 14-14: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 14-14: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 16-16: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Source: Linters/SAST tools