Skip to content

Commit b9024da

Browse files
fix: address PR review workflow/docs/test issues
Assisted-by: GitHub Copilot (GPT-5.3-Codex)
1 parent b7ac1d2 commit b9024da

9 files changed

Lines changed: 51 additions & 14 deletions

File tree

.github/workflows/build.yml

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,15 @@ jobs:
2828
- name: Test with coverage
2929
run: |
3030
coverage erase
31-
coverage run -a tests.py
31+
coverage run -a "$(command -v gff3_QC)" -g example_file/example.gff3 -f example_file/reference.fa -o error.txt
32+
coverage run -a "$(command -v gff3_fix)" -qc_r error.txt -g example_file/example.gff3 -og corrected.gff3
33+
coverage run -a "$(command -v gff3_merge)" -g1 example_file/new_models.gff3 -g2 example_file/reference.gff3 -f example_file/reference.fa -og merged.gff -r merged_report.txt
34+
coverage run -a "$(command -v gff3_merge)" -g1 example_file/new_models.gff3 -g2 example_file/reference.gff3 -f example_file/reference.fa -og merged.gff -u1 example_file/u1.txt -u2 example_file/u2.txt -r merged_report.txt
35+
coverage run -a "$(command -v gff3_merge)" -g1 example_file/new_models.gff3 -g2 example_file/reference.gff3 -f example_file/reference.fa -og merged.gff -u1 example_file/u1.txt -r merged_report.txt
36+
coverage run -a "$(command -v gff3_merge)" -g1 example_file/new_models.gff3 -g2 example_file/reference.gff3 -f example_file/reference.fa -og merged.gff -u2 example_file/u2.txt -r merged_report.txt
37+
coverage run -a "$(command -v gff3_merge)" -g1 example_file/new_models_w_replace.gff3 -g2 example_file/reference.gff3 -f example_file/reference.fa -og merged.gff -r merged_report.txt -noAuto
38+
coverage run -a "$(command -v gff3_sort)" -g example_file/example.gff3 -og example-sorted.gff3
39+
coverage run -a "$(command -v gff3_to_fasta)" -g example_file/example.gff3 -f example_file/reference.fa -st all -d simple -o test_sequences
3240
coverage run -a -m unittest discover -s tests/unit -p "test_*.py"
3341
- name: after success
3442
env:
@@ -57,7 +65,7 @@ jobs:
5765
strategy:
5866
fail-fast: false
5967
matrix:
60-
os: ['ubuntu-24.04', 'windows-2025-vs2026', 'macos-latest']
68+
os: ['ubuntu-24.04', 'windows-latest', 'macos-latest']
6169
runs-on: ${{ matrix.os }}
6270

6371
steps:
@@ -78,7 +86,7 @@ jobs:
7886
strategy:
7987
fail-fast: false
8088
matrix:
81-
os: ['ubuntu-24.04', 'windows-2025-vs2026', 'macos-latest']
89+
os: ['ubuntu-24.04', 'windows-latest', 'macos-latest']
8290
runs-on: ${{ matrix.os }}
8391

8492
steps:
@@ -95,7 +103,7 @@ jobs:
95103
- name: Install wheel and verify CLI entry points
96104
run: |
97105
python -c "import glob,subprocess,sys; wheels=glob.glob('dist/*.whl'); assert wheels, 'No wheel built'; subprocess.check_call([sys.executable, '-m', 'pip', 'install', wheels[0]])"
98-
python -m gff3tool.bin.gff3_sort -h
106+
gff3_sort -h
99107
python -c "import shutil,sys; sys.exit(0 if shutil.which('gff3_sort') else 1)"
100108
101109
build5-Docs-build:
@@ -111,7 +119,6 @@ jobs:
111119
run: |
112120
python -m pip install --upgrade pip
113121
python -m pip install -r docs/requirements.txt
114-
python -m pip install .
115122
- name: Build docs (warnings as errors)
116123
run: |
117124
python -m sphinx -W -b html docs docs/_build/html

.readthedocs.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,3 @@ sphinx:
1111
python:
1212
install:
1313
- requirements: docs/requirements.txt
14-
- method: pip
15-
path: .

docs/HANDOFF.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
- Date: 2026-05-15
55
- Branch: chore/version-updates-followup
66
- Latest commit before this note: 6862a83
7-
- Goal completed: Python 3.14 compatibility reassessment with runtime and packaging checks.
7+
- Goal completed locally: Python 3.14 compatibility reassessment with runtime and packaging checks.
8+
- Note: several follow-up review items remained open after this validation.
89

910
## What Was Validated Under Python 3.14
1011
Environment used:
@@ -40,6 +41,7 @@ Commands run:
4041
### 3) setup.py uses private setuptools fallback path
4142
- setup.py imports setuptools._distutils.command.build in fallback path.
4243
- This works now, but it is a private path and has upgrade fragility risk.
44+
- The BLAST bundle step also needs to be idempotent and safe against archive path traversal.
4345

4446
### 4) Smoke test caveat
4547
- tests.py requires bundled BLAST executables to be available.
@@ -55,6 +57,7 @@ Commands run:
5557
1. Update pyproject.toml license metadata to remove setuptools-deprecated forms.
5658
2. Consider removing private setuptools._distutils fallback usage in setup.py.
5759
3. Keep tests.py BLAST preflight behavior, but ensure local instructions clearly explain BLAST artifact requirements.
60+
4. Resolve the remaining review comments in the workflow, docs, setup, and tests before treating the branch as finished.
5861

5962
## Fast Resume Checklist
6063
1. Confirm branch: chore/version-updates-followup

docs/conf.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
'_build',
6969
'Thumbs.db',
7070
'.DS_Store',
71+
'HANDOFF.md',
7172
# Keep these markdown files for GitHub readability but avoid duplicate
7273
# Sphinx docnames where .rst versions exist.
7374
'Detection-of-GFF3-format-errors.md',

docs/gff3_to_fasta.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Extract sequences from specific regions of genome based on gff file.
55
## Features
66

77
* **Incorporation of [gff3.py](https://github.com/hotdogee/gff3-py)**: `gff3.py` is contributed by [Han Lin](https://github.com/hotdogee) which uses simple data structures to parse a [`GFF3`] file into a structure composed of simple python [`dict`] and [`list`].
8-
* **Validation**: Validate the [GFF3 formatting errors](Detection-of-GFF3-format-errors.rst) utilizing [QC methods](../gff3tool/bin/gff3_QC.py) contributed by the [I5K Workspace@NAL team](https://i5k.nal.usda.gov/). Provide `WARNING` messages for gene models that may have incorrect biological sequences generated because of [`GFF3`] formatting errors.
8+
* **Validation**: Validate the [GFF3 formatting errors](Detection-of-GFF3-format-errors.rst) utilizing [QC methods](gff3_QC.md) contributed by the [I5K Workspace@NAL team](https://i5k.nal.usda.gov/). Provide `WARNING` messages for gene models that may have incorrect biological sequences generated because of [`GFF3`] formatting errors.
99
* **Easy extraction of biological sequences**: Provide options for extracting six types of biological sequences or user-specified type of spliced sequences.
1010
- **`gene`**: Gene sequence for each record in the [`FASTA`] output. Gene or pseudogene features need to be included in the gff file
1111
- **`exon`**: Exon sequence for each record in the [`FASTA`] output. Exon features need to be included in the gff file

gff3tool/lib/id_processor.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ def general_newModel(oldmodel, gff):
193193
newid = oldmodel['attributes']['ID']
194194
if newid in gff.features.keys():
195195
eofindex = len(gff.lines)
196-
newid = eofindex
196+
newid = str(eofindex)
197197
newmodel = newParentModel(oldmodel, newid, gff)
198198
gff.features[newid].append(newmodel)
199199
gff.lines.append(newmodel)

setup.py

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,23 @@ def bundle_blast(project_root, version):
4040

4141
blast_path = path.join(project_root, 'gff3tool', 'lib', 'ncbi-blast+')
4242
blast_file = path.join(blast_path, 'blast.tgz')
43+
blast_bin_path = path.join(blast_path, 'bin')
4344
download_url = 'https://ftp.ncbi.nlm.nih.gov/blast/executables/blast+/{0:s}/{1:s}'.format(
4445
version, archive_name)
4546

47+
if path.exists(blast_bin_path):
48+
return
49+
4650
if not path.exists(blast_path):
4751
mkdir(blast_path)
4852

4953
urlretrieve(download_url, blast_file)
5054

5155
tar = tarfile.open(blast_file, 'r:gz')
52-
tar.extractall(blast_path)
53-
tar.close()
56+
try:
57+
_safe_extract_tar(tar, blast_path)
58+
finally:
59+
tar.close()
5460

5561
extract_path = path.join(blast_path, 'ncbi-blast-{0:s}+'.format(version))
5662
shutil.move(path.join(extract_path, 'bin'), blast_path)
@@ -61,6 +67,26 @@ def bundle_blast(project_root, version):
6167
shutil.rmtree(extract_path)
6268

6369

70+
def _safe_extract_tar(archive, destination):
71+
destination_root = path.realpath(destination)
72+
if not destination_root.endswith(path.sep):
73+
destination_root = destination_root + path.sep
74+
75+
safe_members = []
76+
for member in archive.getmembers():
77+
if path.isabs(member.name):
78+
raise RuntimeError('Refusing to extract absolute tar member: {0:s}'.format(member.name))
79+
if member.issym() or member.islnk():
80+
raise RuntimeError('Refusing to extract linked tar member: {0:s}'.format(member.name))
81+
82+
member_path = path.realpath(path.join(destination, member.name))
83+
if not member_path.startswith(destination_root):
84+
raise RuntimeError('Refusing to extract outside destination: {0:s}'.format(member.name))
85+
safe_members.append(member)
86+
87+
archive.extractall(destination, members=safe_members)
88+
89+
6490
class bdist_wheel(_bdist_wheel):
6591
def finalize_options(self):
6692
_bdist_wheel.finalize_options(self)

tests.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ def assert_fasta_has_header(path: Path) -> None:
108108

109109
def run_command(name: str, args: list[str], expected_files: list[Path]) -> None:
110110
cmd = [resolve_command(args[0]), *args[1:]]
111+
for output_path in expected_files:
112+
remove_if_exists(output_path)
111113

112114
print(f"[RUN] {name}: {' '.join(cmd)}")
113115
result = subprocess.run(cmd, cwd=ROOT)

tests/unit/test_id_processor.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,8 @@ def test_general_new_model_uses_eof_index_when_id_already_exists(self):
224224
id_processor.general_newModel(oldmodel, gff)
225225

226226
new_root = gff.lines[2]
227-
self.assertEqual(new_root["attributes"]["ID"], 2)
228-
self.assertIn(2, gff.features)
227+
self.assertEqual(new_root["attributes"]["ID"], "2")
228+
self.assertIn("2", gff.features)
229229
self.assertTrue(new_root["children"])
230230

231231
def test_new_nreplace_model_replaces_old_model_and_removes_original(self):

0 commit comments

Comments
 (0)