extract f-string backslash to _safe_json for Python 3.10/3.11 compat#11
extract f-string backslash to _safe_json for Python 3.10/3.11 compat#11ypriverol merged 3 commits intobigbio:devfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors inline json.dumps(...).replace(...) usage in HTML-generating report code into a shared helper to maintain Python 3.10/3.11 f-string compatibility when embedding JSON into <script> blocks.
Changes:
- Introduced a
_safe_json()helper inqc_report.pyto serialize JSON and escape</for safe HTML<script>embedding. - Updated QC and workflow comparison report HTML generation to use
_safe_json(...)instead of inlinejson.dumps(...).replace(...). - Updated imports in
workflow_comparison.pyto pull in the new helper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| mokume/reports/workflow_comparison.py | Switches embedded JS data blocks to _safe_json(...) and imports the helper from qc_report. |
| mokume/reports/qc_report.py | Adds _safe_json() and uses it for embedded PCA/t-SNE/correlation JSON payloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| from mokume.reports.qc_report import compute_qc_metrics, _safe_json | ||
| from mokume.core.logger import get_logger | ||
|
|
| import pandas as pd | ||
|
|
||
| from mokume.reports.qc_report import compute_qc_metrics | ||
| from mokume.reports.qc_report import compute_qc_metrics, _safe_json |
| def _safe_json(obj): | ||
| """Serialize to JSON and escape closing script tags for safe HTML embedding.""" | ||
| import json | ||
|
|
||
| return json.dumps(obj).replace("</", "<\\/") | ||
|
|
||
|
|
This is a Python version compatibility issue: Python 3.12+ (PEP 701) allows backslashes inside f-string expressions, but CI runs Python 3.10/3.11 which does not.
Root Cause: 7 instances of
json.dumps(obj).replace('</', '<\\/')inside f-string expressions in:Fix: Extract
json.dumps() + replace()into a new _safe_json() helper function (defined outside f-strings), replace all 7 inline calls with _safe_json(obj).Problem 2: CI not triggering on
devbranchWorkflow trigger only listed
mainanddevelop, butdevelopbranch doesn't exist — the actual development branch isdev.Fix: Replace
developwithdevin .github/workflows/python-app.yml.Problem 3: test_pride_aggregation_comparison KeyError
PRIDE FTP updated the PXD063291.zip structure —
report.tsvmoved from PXD063291/ toPXD063291/DiaNN/, and 13 new TSV files (e.g.report.gg_matrix.tsv) were added. The fuzzy"report" in f.namematch picked up the wrong file.Fix: Use exact
f.name == "report.tsv"matching in download_pride_dataset().