Skip to content

fix(tsfile): version-aware guard for the optional Apache TsFile SDK - #8517

Closed
AxelNoun wants to merge 1 commit into
huggingface:mainfrom
AxelNoun:fix/tsfile-optional-extra-version-aware
Closed

AxelNoun wants to merge 1 commit into
huggingface:mainfrom
AxelNoun:fix/tsfile-optional-extra-version-aware

Conversation

@AxelNoun

@AxelNoun AxelNoun commented Aug 26, 2026 •

Copy link
Copy Markdown

Status (2026-10-07): draft. This description predates #8502, which raised the requirement on main to pyarrow>=24.0.0, and is therefore out of date, in particular the table and Q1. See my comment of 2026-10-07 below for the current status.

Summary

  • Raise a version-aware ImportError when the Apache TsFile SDK is missing: point at pip install 'datasets[tsfile]' only on Python 3.14+, and explain the pyarrow conflict on 3.10 to 3.13.
  • Declare optional extra tsfile>=2.3.0; python_version >= '3.14' (mirrored in test/dev extras) and install it in the py3.14 CI job via datasets[tsfile].
  • Document the .tsfile extension and the 3.14+ co-install constraint; add missing-SDK / message tests (round-trips stay behind importorskip / require_tsfile).

Scope note

This does not fix the empty Format filter reported in #8499. tsfile is not a registered value in the Hub's format tag taxonomy, which accounts for that symptom on its own and is not actionable in this repository. Hub-inspect / streaming file-open failures are also out of scope (separate PR).

Why the extra alone is not enough

Environment markers make pip install 'datasets[tsfile]' succeed while installing nothing on Python 3.10 to 3.13. An error message that only says "install the extra" sends the user in a loop. The guard must be version-aware.

Python SDK pyarrow pin datasets Co-install?
3.10 to 3.13 >=18,<20 >=21 No
3.14+ >=22,<24 >=21 Yes

Question for maintainers (Q1)

Packaging policy: keep the silent environment marker (current default in this PR), or drop the marker so pip install 'datasets[tsfile]' fails loudly with ResolutionImpossible on 3.10 to 3.13?

This is a project packaging preference; the diff keeps the marker and will follow whatever you prefer.

Supersedes

Supersedes closed #8514. That branch's commit message linked the unguarded import to the Format facet; this link has since been refuted, and the correction is in a comment on that PR. This PR is a fresh branch from main with the version-aware guard.

Test plan

  • pytest tests/packaged_modules/test_tsfile_guard.py (2 passed)
  • ruff check on touched Python files
  • tests/packaged_modules/test_tsfile.py round-trips (py3.14 CI / importorskip locally)

Refs

The TsFile builder imported the third-party `tsfile` SDK inside
`_scan_metadata` without a guard, surfacing a bare `ModuleNotFoundError`
to users and to the dataset-viewer workers.

Scope note: this is a usability and packaging fix. It does not fix the
Format filter reported in huggingface#8499 — `tsfile` is not registered in the Hub's
format taxonomy, which accounts for that symptom on its own and is not
actionable in this repository. It also does not fix Hub-inspect / streaming
file-open failures (separate PR).

On Python 3.10-3.13 the SDK cannot co-install with `datasets`
(pyarrow<20 vs pyarrow>=21); the ImportError must say so, because an
environment-marked `datasets[tsfile]` extra is silently empty there.

- raise a version-aware ImportError (point at `datasets[tsfile]` only on 3.14+)
- declare `tsfile>=2.3.0; python_version >= '3.14'` and mirror it in test extras
- document the `.tsfile` extension and the 3.14+ co-install constraint
- add missing-SDK and message tests; keep round-trips behind `require_tsfile`

Refs huggingface#8499, huggingface#8160

Co-authored-by: Cursor <cursoragent@cursor.com>
@AxelNoun
AxelNoun marked this pull request as draft October 7, 2026 10:54
@AxelNoun

AxelNoun commented Oct 7, 2026

Copy link
Copy Markdown
Author

Status (2026-10-07): I have converted this PR to a draft. Its premise no longer holds on main, and the branch now conflicts with main.

  1. Since Add support for datasets backed by vortex file format #8502, main requires pyarrow>=24.0.0 (first released in datasets 5.1.0), and the Python 3.14 CI job installs the SDK with --no-deps. tsfile 2.3.0 to 2.4.0 require pyarrow<20 on Python 3.10 to 3.13 and pyarrow<24 on Python 3.14. Against current main, the extra proposed here (tsfile>=2.3.0; python_version >= '3.14') would therefore install the SDK on no Python version: it is empty below 3.14 because of the marker, and it cannot be resolved on 3.14 unless pre-releases are allowed (uv pip install --dry-run "datasets==5.1.0" "tsfile>=2.3.0" fails there). The version-aware error message and the "Python 3.14+" wording in the documentation are no longer accurate either.
  2. Fix/relax pyarrow constraint apache/tsfile#922 removed the upper bounds on pyarrow. On PyPI, only the pre-releases 2.5.0.dev0 and 2.5.0.dev1 include this change so far; the latest stable release is 2.4.0.

Question for the maintainers before any rework: is an import guard or a datasets[tsfile] extra wanted for this builder at all? For reference, on main the vortex, lance, hdf5 and iceberg builders import their SDK without a guard, only iceberg among them has an extra, and #8307 already proposes a plain guard. If neither is wanted, this PR can be closed.

@AxelNoun

Copy link
Copy Markdown
Author

Closing. The version-aware guard and the Python-version marker proposed here worked around the pyarrow upper bounds of tsfile 2.3.0 to 2.4.0. apache/tsfile#922 removed those bounds, and tsfile 2.5.0 (14 October) only sets lower bounds (pyarrow>=16, >=18 on Python 3.13, >=22 on 3.14). datasets==5.1.0 and tsfile==2.5.0 now resolve together on Python 3.10, 3.13 and 3.14 (uv pip compile, Linux x86_64), so the premise of this PR no longer holds. If a plain extra or import guard is wanted for this builder, that would be a separate, smaller change.

I used Claude Code to prepare this comment.

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.

1 participant