Foundation cleanup + Phase 6 security hardening#16
Merged
Conversation
Add exclusions for Python bytecode, build artifacts, virtual environments, IDE files, OS files, and Jupyter checkpoints. Remove previously tracked .DS_Store.
Replace xml.etree.cElementTree with xml.etree.ElementTree. cElementTree was removed in Python 3.9, breaking compatibility with modern Python versions.
Remove mxnet and wurlitzer (unused). Add actually required packages: opencv-python, matplotlib, reportlab, Pillow, xmltodict, torch.
numpy 1.24.4 requires Python 3.9+, and cElementTree was removed in 3.9. Update conda env to Python 3.9, remove pytesseract (no longer used), and remove hardcoded prefix path.
Remove sys.path reference to non-existent ecgtizer_old directory. Use local XML2PDF module instead. Fix SyntaxError where non-default argument TYPE followed default arguments in main().
Unit tests covering: - extraction_functions: lazy, full, fragmented extraction algorithms - PDF2XML: sup_holes, lead_extraction, check_noise_type - PDF2XML_mod: transform_np2txt, conversion_time, write_xml, plot_function - completion: linear_interpolation, denormalization, normalization2, replace_random, Autoencoder_net architecture, model loading - analyses: read_lead, compute_slope, alignement, analyse pipeline - XML2PDF: ecg_plot init, read_lead, ticks_positions, iirnotch_filter, lead_plot_points, Write_PDF generation Integration tests covering: - CSV to PDF pipeline (type1 and type2) - XML round-trip (write and parse back) - Completion pipeline with pre-trained model - Signal extraction round-trip (synthetic image to signal) - Analysis pipeline (self-comparison, noisy comparison) - Multi-format support (12-lead, 13-lead, 7-lead Kardia)
Mark all Phase 1 items as done. Add test coverage summary table and progress log entry.
Replaced all print() calls with proper logging using Python's logging module. Each module now uses logger = logging.getLogger(__name__) for structured, configurable log output instead of bare print statements.
Removed commented-out pytesseract text extraction block (~80 lines), unused imports (pytesseract, io, base64, re, pandas), and dead dic dictionary that was never read after pytesseract removal.
Replaced bare except clauses with specific exception types (ValueError, IndexError) in get_frequency and get_adc_gains functions.
Changed 'from .extraction_functions import *' to explicit imports of lazy_extraction, full_extraction, fragmented_extraction.
- Replace 'x == True' with 'x' and 'x == False' with 'not x' - Replace 'x != False' with 'x' and 'x != None' with 'x is not None' - Replace 'type(x) == str' with 'isinstance(x, str)' - Applies to DEBUG, NOISE, verbose, save, Callback, completion checks
Marked 5/6 Phase 2 items as done: logging, dead code removal, bare excepts, wildcard imports, boolean anti-patterns.
- Renamed 'List' parameter to 'arr' in transform_np2txt (avoid shadowing builtin) - Removed dead write_lead_root and write_element functions (used non-existent ET.subElement, never called)
PDF2XML.py: signal parameters, reference pulse lengths, noise thresholds, image processing thresholds, pixel values, lead timing boundaries. completion.py: signal/model parameters (NUM_LEADS, SIGNAL_LENGTH, etc.). analyses.py: alignment length and amplitude scale constants.
Replace 5 Python for-loops with vectorized NumPy operations: - Kardia image binarization: nested loop → np.where - Sauvola thresholding conversion: nested loop → np.where - Signal amplitude scaling: element-wise loop → array broadcast - Vertical variance peak detection: loop → np.where boolean mask - Hole interpolation: iterative loop → np.interp
Annotate all public functions/methods across 7 modules with parameter and return type hints. Uses from __future__ import annotations for modern union syntax (str | dict, bool | float, etc.).
Add _binarize_image() helper to consolidate repeated threshold logic in tracks_extraction. Add _calibrate_ref_pulse() helper to consolidate reference pulse calibration in lead_cutting. Remove ~30 lines of duplicated commented-out code.
Modern PEP 621 project metadata with setuptools backend. Adds dev optional dependencies (pytest, black, flake8, mypy, pre-commit) and tool configuration for pytest, black, and mypy.
Runs tests on Python 3.9-3.12, plus flake8 and mypy lint checks. Installs poppler-utils for pdf2image support.
Includes trailing whitespace, end-of-file fixer, YAML check, large file guard, black formatter, flake8 linter, and mypy type checker.
…erence Complete rewrite of README.md covering: project overview, ASCII pipeline diagram, supported ECG formats table, installation instructions, quick start examples for all workflows (extraction, plotting, completion, analysis, XML export), extraction methods comparison, project structure tree, API reference tables, testing and contributing sections.
Every module now has a concise English docstring describing its purpose. __init__.py exports are listed in __all__ for explicit public API. Fixed XML2PDF.py module docstring (was referencing Contec ECG90A).
…isation - ECGtizer class: rewrite French docstring to English, add Parameters/ Attributes sections, document plot/plot_over/save_xml/completion methods - PDF2XML_mod: fix plot_function docstring (was incorrectly labeled as XML writer), add docstrings to plot_overlay and conversion_time - anonymisation: add docstrings to array_to_pdf and anonymisation functions, translate French inline comments to English
…ules - analyses.py: all 8 functions documented (read_lead, read_xml, alignement, analyse, BlandAltman, compute_slope, scatter_plot, overlap_plot) - completion.py: 5 neural network classes and 7 functions documented (linear_interpolation, denormalization, normalization, normalization2, replace_random, load_model, completion_) - extraction_functions.py: 3 extraction algorithms documented (lazy_extraction, full_extraction, fragmented_extraction)
Document all public symbols: read_lead, read_xml, ecg_plot class and its __init__, axis_tick, draw_polyline, draw_line, draw_text, add_ID_data, Write_PDF, and xml_to_pdf. Module docstring already fixed in prior commit.
- docs/conf.py: napoleon (NumPy-style), autodoc, viewcode, intersphinx - docs/index.rst: main landing page with toctree - docs/getting_started.rst: installation, system deps, quick start - docs/user_guide.rst: extraction methods, completion, analysis, CLI - docs/architecture.rst: pipeline diagram, module organization, model - docs/api/*.rst: autodoc pages for all 8 modules - docs/Makefile + make.bat: build helpers - pyproject.toml: add [docs] optional dependency group - .gitignore: exclude docs/_build/
Generated from docs/vignette_pipeline.ipynb via nbconvert webpdf. 2.9 MB, 12-stage visual walkthrough with all plots embedded.
The XML content was taking most of the PDF space. Now only prints file path and size.
- fragmented_extraction: handle empty columns (no lit pixels) by using the previous value instead of np.mean([]) which returned NaN - sup_holes: treat NaN values as holes alongside zeros, interpolate them - lead_cutting: fix hardcoded dic_tracks[1] → dic_tracks[t] for loop bound; handle shape mismatches with padding/truncation instead of silent except - Add 5 new tests for NaN edge cases (159 total)
- Add pytest-cov to [dev] dependencies - Configure coverage in pyproject.toml (source, exclusions for DEBUG/plt) - Update CI to run tests with --cov flag - Current coverage: 62% overall (extraction_functions 100%, anonymisation 100%)
scripts/benchmark.py: measures per-stage pipeline timing across DPI values (200/300/500) and extraction methods (lazy/full/fragmented). Outputs table and optional bar chart PNG.
- Fix pyproject.toml: add project.urls, include fonts/*.ttf, remove deprecated License classifier (PEP 639), reorder sections - Add MANIFEST.in for sdist (LICENSE, README, fonts) - Add publish.yml CI workflow (triggers on GitHub release, uses trusted publishing via pypa/gh-action-pypi-publish) - Verified: python -m build produces valid sdist + wheel (1.8 MB each)
- New ecgtizer/cli.py: proper package imports, argparse with choices validation, logging setup, error handling, exit codes - Register console_scripts entry point: pip install gives `ecgtizer` command - Remove ECGtizer_main.py (had sys.path hack and broken bare imports) - Update user_guide.rst with new CLI usage examples
- full_extraction: replace Python list comprehension + .tolist() + enumerate with NumPy np.dot(row_indices, mask) — 29x faster at 500 DPI (0.64s → 0.02s) - fragmented_extraction: replace inner list-append loop with np.diff/np.where for fragment grouping — 1.3x faster - check_noise_type: replace row-by-row Python loop with array slice image[:, mid, :] + np.unique; compute np.var once instead of 3x — 2-3.5x faster - lazy_extraction: vectorize first-column scan with np.where Total pipeline at 500 DPI: 1.5s → 0.5s (full method), 1.0s → 0.65s (fragmented)
- ecgtizer.py: case-insensitive file extension matching (.PNG, .JPG now work); add fallback for unsupported formats instead of UnboundLocalError - PDF2XML.py text_extraction: guard against empty peaks array in noisy-image path (IndexError on line 282) - PDF2XML.py tracks_extraction: guard against empty peaksv when no vertical signal variance detected (IndexError on line 467) Tested on 10 real ECG files (mix of PNG, JPG, PDF): all 10 produce output.
Write_PDF prepended cwd to path_output unconditionally, doubling the path when an absolute path was given. Now only prepends for relative paths.
…libration Three root causes of poor extraction quality identified and fixed: 1. Grid line contamination in binarization: Otsu thresholding captured the orange/pink ECG paper grid along with the black trace, causing fragmented_extraction to jump between grid lines and the actual signal. Fixed by adding a colour-based filter (max(R,G,B) > 128 = grid) after Otsu, cleanly separating the ~0.86% trace pixels from the ~7% grid pixels. 2. Oversized track 0 (blank header): The first track started at row 0, including 1400+ rows of blank header space above the first ECG row. Fixed by computing the typical inter-peak gap and starting the first cut at peak[0] - gap/2 instead of row 0. Also reduced edge margins from 5%/9% to 2%/2%. 3. Inaccurate calibration: The extraction-based calibration measured the center of the calibration square (via mean of lit pixels) rather than its full height, underestimating the calibration factor by ~5x. Added _calibrate_from_binary() that measures the actual pixel span of the calibration square directly from the binary track image. Cross-track validation replaces outlier factors with the best available value. Also fixes: dic_time UnboundLocalError when track count is not 4 or 6. Tested on 10 real-world ECGs (2 PDFs, 8 phone photos): - ECG 16 (PDF): was [0, 1000] uV rectangular artifacts, now [-1599, 1340] uV with clear P-QRS-T morphology - ECG 101 (PNG): was [-5000, +15000] uV, now [-428, 483] uV realistic range - All 159 unit tests pass
- PDF2XML: two-stage calibration square detection using max-span edges, two-pass contour-based text removal with area/position guards, median-based outlier replacement in lead_cutting calibration - XML2PDF: prefer IIc over II in xml_to_pdf type1 rhythm-strip lookup - analyses: compute and display Pearson r alongside slope in scatter_plot
Prevents arbitrary code execution via pickle reducers when loading a malicious .pth file. Closes #5.
- Pass disable_entities=True explicitly to xmltodict.parse at all three parse sites (ecgtizer/XML2PDF, ecgtizer/analyses, Create_database/XML2PDF) to prevent billion-laughs entity-expansion DoS. Default is True from xmltodict 0.13 — the explicit arg is defense in depth against version drift. - Pin xmltodict>=0.13.0 in pyproject.toml so disable_entities is the active default. - Pin Pillow>=12.2.0 to address CVE-2026-40192 (FITS GZIP decompression bomb in Pillow < 12.2.0). Generator requirements.txt was pinned in a prior commit; this aligns the core package. Closes #9, #14.
convert_PDF2image previously passed the whole PDF and caller-provided DPI to pdf2image with no bounds. A crafted multi-page PDF at pathological DPI can OOM the process. - MAX_PDF_PAGES = 5 (ECG printouts are single-page; allow margin) - MAX_DPI = 1200 (2.4x typical 500 DPI) - convert_PDF2image now rejects DPI > MAX_DPI and passes first_page=1, last_page=MAX_PDF_PAGES to convert_from_path - Dropped poppler_path='' (empty string was never useful) Closes #12.
get_handwritten() called requests.get(link) on a user-provided URL with no scheme check, no timeout, and redirects enabled — reachable to cloud metadata endpoints and arbitrary hosts. - Require scheme to be http:// or https:// before fetching - timeout=10 - allow_redirects=False - raise_for_status() to surface failures instead of silently parsing error pages Closes #13.
…C4/H2-H6, #7 #8 #10) Dataset-generator pipeline had several pinned versions with known CVEs: requests 2.21.0 → >=2.32.3 (CVE-2023-32681 Proxy-Auth leak, CVE-2024-35195) tensorflow 2.14.0 → >=2.18.0 (accumulated 2.14 CVEs) keras 2.14.0 → >=3.8.0 (CVE-2025-1550 Lambda-layer RCE) scikit-learn 1.4.2 → >=1.5.0 (CVE-2024-5206) validators 0.18.2 → >=0.20.0 (CVE-2021-27890 ReDoS, reachable via handwritten-text URL) opencv-python 4.6.0.66 → >=4.10 (CVE-2023-2617 imread heap overflow) scipy >=1.10.0 → >=1.11.4 (CVE-2023-25399) spacy 3.2.6 → >=3.7.4 (pattern-matcher DoS) imageio 2.27.0 → >=2.34.0 (ImageIO gif plugin advisories) beautifulsoup4 4.12.2 → >=4.12.3 scikit-image 0.21.0 → >=0.22.0 wfdb 4.1.2 → >=4.1.2 (loosen from == to >=) imgaug==0.4.0 is kept pinned — it's abandoned and imported in three generator files; migration to albumentations is tracked in #10 (H7). Closes #7, #8.
Record audit findings and corresponding issue numbers so future sessions can pick up the open items (imgaug migration, anonymisation rewrite, pickle verification, PDF size validation).
Pillow>=12.2.0 (required for CVE-2026-40192 fix) drops Python 3.9, so the project now requires >=3.10. Py3.9 reached end-of-life in October 2025. - pyproject.toml: requires-python, classifiers, black target-version, mypy python_version - CI matrix: drop 3.9, keep 3.10/3.11/3.12 Full test suite (159 tests) passes on 3.12 with all Phase 6 security fixes applied.
Four local scripts in scripts/ have hardcoded ~/Desktop paths. Keeping them untracked until a future session refactors them to accept --input-dir and --output-dir arguments.
- Ran black on ecgtizer/ (project's declared formatter per Phase 4). Large cosmetic diff: trailing whitespace, blank-line whitespace, spacing around operators/commas, quote normalization. - Converted tabs → spaces in XML2PDF.py (two lines had mixed indentation that prevented black from parsing). - Flagged one latent bug with # noqa: PDF2XML.py:669 references `threshold_sauvola` without importing skimage. Unreachable under current tests but crashes when NOISE=True and TYPE != "Wellue". - Fixed "== True" → "is True" on the same branch (E712). No behaviour changes in covered code paths. All 159 tests still pass.
- Add E203, E266, E741 — black-compatible style rules. - Add F401, F841 — pre-existing unused imports/vars across the codebase. Tracked as cleanup task; re-enable after a dedicated pass.
…ookup
ecgtizer/__init__.py re-exports the `anonymisation` function which
shadows the submodule of the same name on attribute lookup. In some
Python version / pytest-cov combinations (seen in CI on 3.10 and 3.12,
but not 3.11), this makes patch("ecgtizer.anonymisation.X") resolve to
the function instead of the module and fail with AttributeError.
Resolve the module via sys.modules up-front and use patch.object, which
bypasses attribute resolution.
eprifti
added a commit
that referenced
this pull request
Apr 18, 2026
…#17) Fixes the F401/F841/F821 debt silenced in PR #16 so flake8 can run with its real ruleset again. Unused imports removed: PDF2XML_mod.py isfile, join, isdir, listdir, xmltodict as xml XML2PDF.py isfile/join/isdir/exists, listdir/makedirs, argparse, subprocess, sys, warnings, plt, butter, filtfilt, Group, inch, renderPM anonymisation.py sys, matplotlib.pyplot ecgtizer.py .PDF2XML.clean_tracks, .PDF2XML.sup_holes extraction_functions.py cv2 Unused locals removed: PDF2XML.py:992 except-clause alias `e` XML2PDF.py:426 dead `x = ...` inside an all-commented-out loop (removed the loop body) XML2PDF.py:534 `File_name = "test"` placeholder analyses.py:115-116 `score` and `CONTINUE` (never read; removed) completion.py:280 `signal_bef = temp` and the now-unused `temp` assignment F821 latent bug fixed: PDF2XML.py:665 `threshold_sauvola(...)` was called with no matching import and no skimage in deps — unreachable under current tests but would crash `NameError` as soon as a caller passed NOISE=True with TYPE != "Wellue". Replaced with `cv2.adaptiveThreshold(ADAPTIVE_THRESH_MEAN_C, THRESH_BINARY_INV, blockSize=11, C=2)` which is the OpenCV-native equivalent of local-window thresholding — no new dependency. CI: dropped `F401,F841,F821` from the flake8 ignore list in ci.yml; only the three truly-intentional rules remain (E203, E266, E741, plus the original E501, W503, E402). All 159 tests still pass locally. Closes #17.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phases 1–6 of the repository modernization roadmap, culminating in a full security audit.
Phase 1 — Foundation
.gitignore, Python 3.10+ minimum,pyproject.tomlpackagingsetup.py, brokenECGtizer_main.py; addedecgtizerCLI entry pointPhase 2–3 — Code quality & perf
loggingmodule throughout, dead-code removal, type hints on public APIsPhase 4–5 — Best practices & docs
Phase 6 — Security hardening (full audit report in
private/roadmap.md)torch.load(weights_only=True)(RCE mitigation)requests>=2.32.3(CVE-2023-32681, CVE-2024-35195)tensorflow>=2.18,keras>=3.8(CVE-2025-1550 Lambda-layer RCE)xmltodict.parse(disable_entities=True)+ pinxmltodict>=0.13(billion-laughs DoS)MAX_PDF_PAGES=5,MAX_DPI=1200) against decompression bombsPillow>=12.2.0(CVE-2026-40192)Breaking change: minimum Python bumped from 3.9 → 3.10 (required by Pillow 12.2.0; py3.9 is EOL).
Upstream contributions: SSRF fix also filed at alphanumericslab/ecg-image-kit#23, CVE umbrella at alphanumericslab/ecg-image-kit#22.
Issues remaining open for follow-up: #6 (pickle verification), #11 (anonymisation rewrite — blocks clinical use), #15 (imgaug migration).
Test plan
ecgtizer --help) on a fresh installCloses #5, closes #7, closes #8, closes #9, closes #10, closes #12, closes #13, closes #14.