Skip to content

fix: reuse converted release datasets - #58

Closed
SilvioGiancola wants to merge 1 commit into
devfrom
feat/release-verification-tests
Closed

fix: reuse converted release datasets#58
SilvioGiancola wants to merge 1 commit into
devfrom
feat/release-verification-tests

Conversation

@SilvioGiancola

Copy link
Copy Markdown
Collaborator

Summary

  • reuse an existing split JSON before downloading Parquet/WebDataset shards from Hugging Face
  • expose whether a download was skipped and cover the cache-hit behavior with a regression test
  • align classification release fixtures with per-split media roots and excluded dataset labels
  • normalize detected CUDA versions before selecting PyTorch wheels
  • document the reused JSON behavior

Validation

  • cache-hit smoke test passed
  • Python compilation passed for the modified transfer module and tests
  • git diff --check passed
  • full pytest suite not run because pytest is unavailable in the current shell

API impact

Backward-compatible: Parquet download results now include download_skipped. Existing converted JSON files are reused.

Copilot AI lite review requested due to automatic review settings August 30, 2026 13:07
@SilvioGiancola

Copy link
Copy Markdown
Collaborator Author

Superseded by a PR from the fresh fix/skip-hf-download-existing-json branch, as requested.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves Hugging Face dataset split handling by reusing already-converted per-split JSON outputs (avoiding redundant Parquet/WebDataset downloads), updates release test fixtures to match the per-split media extraction layout and dataset label exclusions, and fixes CUDA version normalization for selecting PyTorch wheel indexes. It also documents the new “reuse existing JSON” behavior.

Changes:

  • Add a Parquet cache-hit path to skip HF download/conversion when <output>/<revision>/<split>/<split>.json already exists, and surface this via download_skipped plus a regression test.
  • Align classification release fixtures with per-split source_path roots and exclude labels that are hardcoded in ClassificationDataset.
  • Normalize detected CUDA versions (e.g., 13.0cu130) before selecting the PyTorch wheel index, and document the reuse behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
opensportslib/tools/hf_transfer.py Adds Parquet cache-hit short-circuit and download_skipped result field.
tests/test_hf_transfer_tools.py Adds regression coverage for “existing JSON skips download/conversion” and asserts download_skipped.
tests/release/test_classification_release.py Updates release dataset fixture to use per-split media roots and exclude hardcoded dataset labels.
opensportslib/setup/setup.py Normalizes detected CUDA version into a pip wheel tag before selecting PyTorch index.
README.md Documents JSON reuse behavior for Parquet/WebDataset downloads.
tests/release/README.md Documents Parquet cache-hit behavior in release test documentation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +63 to +68
# get_cuda_version() returns the dotted version reported by nvidia-smi
# (e.g. "13.0"), not a pip wheel tag (e.g. "cu130") -- convert before
# comparing, the same way install_pyg() below already does.
detected_tag = f"cu{CUDA_VERSION.replace('.', '')}" if CUDA_VERSION else None
if detected_tag in CUDA_SUPPORT:
cuda = detected_tag
Comment on lines +221 to +245
output_json_path = Path(output_dir) / f"{cleaned_split}.json"
if output_json_path.is_file():
_emit_progress(
progress_cb,
f"JSON already exists at {output_json_path}; skipping Parquet/WebDataset download and conversion.",
)
return {
"repo_id": cleaned_repo_id,
"revision": cleaned_revision,
"split": cleaned_split,
"folder_path": cleaned_split,
"output_dir": output_dir,
"json_path": str(output_json_path),
"source": "parquet_split",
"download_kind": "parquet",
"downloaded_file_count": 0,
"download_skipped": True,
"extracted_media": True,
"extracted_media_count": 0,
"hf_source_metadata": {
"repo_id": cleaned_repo_id,
"branch": cleaned_revision,
"split": cleaned_split,
},
}
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.

2 participants