clean_path() normalises URL paths with os.path.normpath, breaking every URL-rooted dataset on Windows
维护者通常 3 天内回复
还没有人认领这个 Issue。
评估
调研方向
从 pyhealth/datasets/base_dataset.py 中的 clean_path() 入手,检查其 URL 处理逻辑以及 issue 中描述的三个调用点。为 URL 和本地路径场景添加一个无需联网的单元测试,然后运行该测试。完成标准是 URL 分隔符和缓存源指纹在各个平台上保持一致,且测试通过。
由索引模型根据 Issue 内容生成。
描述
clean_path() normalises the path component of a URL with os.path.normpath, which on Windows rewrites every / as \:
# pyhealth/datasets/base_dataset.py
def clean_path(path: str) -> str:
"""Clean a path string."""
if is_url(path):
parsed = urlparse(path)
cleaned_path = os.path.normpath(parsed.path)
# Rebuild the full URL
return urlunparse(parsed._replace(path=cleaned_path))
urlunparse then puts those backslashes back into the URL, so no dataset with a URL root can be read on Windows.
Reproduction
Checked on master at 4a173f7, Windows 11, Python 3.13.5.
>>> from pyhealth.datasets.base_dataset import clean_path
>>> clean_path("https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III/PATIENTS.csv.gz")
'https://storage.googleapis.com/\\pyhealth\\Synthetic_MIMIC-III\\PATIENTS.csv.gz'
End to end, any URL-rooted dataset fails. The three tests in tests/core/test_processor_schemas.py, which build a MIMIC3Dataset on https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III, all fail the same way:
FileNotFoundError: Neither path exists:
https://storage.googleapis.com/\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv.gz or
https://storage.googleapis.com/\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv
pyhealth\datasets\base_dataset.py:203: FileNotFoundError
The error surfaces in _csv_tsv_gz_path, but the mangling happens earlier, in clean_path.
Reach
clean_path has three call sites, and the bug is not limited to reading:
_scan_table, line 847 — the main read path for every table.- the join path, line 867 — same, for joined tables.
_resolve_cache_dir, line 540 —_source_fingerprint(clean_path(f"{self.root}/{file_path}"))goes intokey["sources"], which is hashed into the dataset's cache UUID.
The third one has a consequence beyond Windows being broken: for the same dataset, the cache key is platform-dependent. With the synthetic MIMIC-III root and tables=["patients"]:
| platform | source recorded in the key | resulting dataset_uuid |
|---|---|---|
| Windows | https://storage.googleapis.com/\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv.gz |
8cd7c9be-4dfd-5c81-a105-fdc4042b34dc |
| POSIX | https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III/PATIENTS.csv.gz |
6c4bdad7-c16e-5d84-b49f-54a704600a3d |
So a cache directory built on Linux is not found by a Windows run of the same code, and vice versa. That matters for anyone sharing a cache across machines, and it is invisible: nothing reports a miss, the dataset is simply rebuilt under another UUID.
The same substitution also applies to non-HTTP schemes, since is_url only requires a scheme and a netloc: s3://bucket/key/file.csv.gz becomes s3://bucket/\key\file.csv.gz.
Fix
A URL path is always POSIX, so it should be normalised with posixpath, which behaves identically on every platform:
import posixpath
def clean_path(path: str) -> str:
"""Clean a path string."""
if is_url(path):
parsed = urlparse(path)
# A URL path is POSIX whatever the host OS: os.path.normpath would
# rewrite the separators as backslashes on Windows.
cleaned_path = posixpath.normpath(parsed.path)
return urlunparse(parsed._replace(path=cleaned_path))
else:
return str(Path(path).expanduser().resolve())
Verified on the same machine:
| input | current | with posixpath |
|---|---|---|
.../Synthetic_MIMIC-III/PATIENTS.csv.gz |
.../\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv.gz |
unchanged, correct |
.../Synthetic_MIMIC-III//PATIENTS.csv |
.../\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv |
.../Synthetic_MIMIC-III/PATIENTS.csv |
https://example.org/a/b/../c.csv |
https://example.org/\a\c.csv |
https://example.org/a/c.csv |
s3://bucket/key/file.csv.gz |
s3://bucket/\key\file.csv.gz |
unchanged, correct |
Note that the fix changes the cache key of URL-rooted datasets on Windows (to the POSIX one, which is the correct and portable value), so existing Windows caches will be rebuilt once. That seems preferable to keeping them keyed on a malformed URL, but say so if you would rather handle it differently.
Two questions before I send a PR:
- Would you like the fix to come with a unit test over
clean_pathfor the four cases above? It needs no network, unliketest_processor_schemas.py. line 113dates fromabc57230(1 May 2025), so the behaviour has been there for a while, and CI apparently runs on Linux only. Would a Windows job on a small subset of the test suite be welcome, or is that out of scope here?
Happy to open the PR either way.
- 主要语言
- Python
- 星标
- 1.7k
- 派生
- 803
- 平均合并
- 5 天 9 小时
- 30 天内合并 PR
- 50
环境准备
- 没有 Dockerfile 或 Docker Compose 文件
- 没有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
sunlabuiuc/PyHealth 的其他 Issue
-
难度 4/5 3-5 天 新手友好度 30/100
sunlabuiuc/PyHealth#1297 ·
维护者通常 3 天内回复
-
难度 4/5 3-5 天 新手友好度 22/100
sunlabuiuc/PyHealth#1295 ·
维护者通常 3 天内回复
-
难度 3/5 1-2 天 新手友好度 42/100
sunlabuiuc/PyHealth#1294 · 1 条评论 ·
维护者通常 3 天内回复
-
Bug: SHHS sleep staging can raise IndexError when signal epochs exceed available labels Copy title可能已有人在做 @cpruijsen 于 18 天前认领。 未关闭
难度 2/5 1-3 小时 新手友好度 25/100
sunlabuiuc/PyHealth#1283 ·
维护者通常 3 天内回复
-
Internal transfers are counted as readmissions可能已有人在做 @thebean27 于 21 天前认领。 未关闭
难度 4/5 3-5 天 新手友好度 35/100
sunlabuiuc/PyHealth#1269 · 3 条评论 ·
维护者通常 3 天内回复
查看 sunlabuiuc/PyHealth 的全部 Issue
相似的 Issue
-
难度 2/5 1-3 小时 新手友好度 67/100
维护者通常 2 天内回复
-
难度 2/5 1-3 小时 新手友好度 68/100
letsencrypt/cp-cps#361 ·
-
难度 2/5 1-3 小时 新手友好度 65/100
-
CI: startup_failure because callers grant fewer permissions than ledger-app-workflows@v1 declares可能已有人在做 @maykonmichel 今天认领。 未关闭
难度 2/5 1-3 小时 新手友好度 76/100
LedgerHQ/app-boilerplate-rust#227 ·
-
难度 2/5 1-3 小时 新手友好度 76/100
linuxmint/mintupdate#1107 ·