Skip to content

fix(dataset): resolve relative sample files on Windows and filesystem URIs (#4502)#4525

Open
Anai-Guo wants to merge 1 commit into
UKGovernmentBEIS:mainfrom
Anai-Guo:fix/dataset-windows-uri-paths
Open

fix(dataset): resolve relative sample files on Windows and filesystem URIs (#4502)#4525
Anai-Guo wants to merge 1 commit into
UKGovernmentBEIS:mainfrom
Anai-Guo:fix/dataset-windows-uri-paths

Conversation

@Anai-Guo

Copy link
Copy Markdown
Contributor

Fixes #4502.

Problem

json_dataset() and csv_dataset() store Dataset.location with os.path.abspath(), while resolve_sample_files() derives the parent directory by splitting that value on the fsspec filesystem separator:

fs = filesystem(dataset.location)
parent_dir = fs.sep.join(dataset.location.split(fs.sep)[:-1])

Two things go wrong:

  1. Windowsabspath() returns backslashes but LocalFileSystem.sep is /, so the split finds no separator, parent_dir becomes "", and the candidate path /images/ballons.png does not exist — the sample file is silently left relative.
  2. Filesystem URIsos.path.abspath() treats file:// / s3:// as local path strings and corrupts the location before any resolution happens.

Verification

Reproduced and verified on Windows 11 / Python 3.12 against the real package.

Before (both fail):

FAILED test_dataset_image_paths - AssertionError: assert WindowsPath('images/ballons.png').exists()
FAILED test_dataset_image_paths_file_uri - AssertionError
  location = 'D:\...\file:\D:\...\images.jsonl'   # <- URI corrupted by abspath()

After: both pass.

Fix

Use the filesystem-aware helpers that already exist in inspect_ai._util.file:

  • absolute_file_path() for the location — it preserves URI schemes and only resolves genuinely relative local paths.
  • dirname() for the parent directory — it is protocol- and separator-aware (verified against s3://b/sub/d.jsonls3://b/sub, file:///c/x/d.jsonlfile:///c/x, C:\x\d.jsonlC:\x).

import os becomes unused in both readers and is removed.

Adds a regression test (test_dataset_image_paths_file_uri) covering a file:// dataset location; the pre-existing test_dataset_image_paths covers the Windows local-path case and now passes on Windows.

Verified no regression in the plain csv_dataset / json_dataset / file_dataset read paths.

🤖 Generated with Claude Code

… URIs (UKGovernmentBEIS#4502)

`json_dataset()` / `csv_dataset()` stored `Dataset.location` with
`os.path.abspath()`, while `resolve_sample_files()` derived the parent
directory by splitting that value on the fsspec filesystem separator.

Two problems follow:

* On Windows `abspath()` produces backslashes but `LocalFileSystem.sep`
  is `/`, so the split yields no parent and relative sample files were
  left unresolved.
* `os.path.abspath()` treats `file://`, `s3://` and other filesystem
  URIs as local path strings, corrupting the stored location.

Use the existing filesystem-aware helpers instead: `absolute_file_path()`
to build the location (it preserves URI schemes and only resolves genuine
relative paths) and `dirname()` to derive the parent directory.

Adds a regression test covering a `file://` dataset location.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dataset relative files fail on Windows and filesystem URIs

1 participant