fix: reuse converted release datasets - #59
Merged
Conversation
… related tests and documentation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Hugging Face transfer utilities to reuse already-converted per-split JSON outputs (avoiding redundant Parquet/WebDataset downloads), exposes whether a download was skipped in the returned result, and aligns release test fixtures and documentation with the per-split output layout. It also normalizes detected CUDA versions into PyTorch wheel tags when selecting the PyTorch index URL.
Changes:
- Reuse an existing
<split>.jsonfor Parquet/WebDataset transfers and expose adownload_skippedflag (plus regression coverage). - Adjust classification release fixtures to (a) exclude dataset-hardcoded labels and (b) resolve
source_pathper split for sharded datasets. - Normalize CUDA version strings (e.g.,
"13.0"→"cu130") before selecting PyTorch wheel indexes, and document the JSON reuse behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
opensportslib/tools/hf_transfer.py |
Adds a cache-hit fast path for Parquet split downloads and includes download_skipped in results. |
tests/test_hf_transfer_tools.py |
Adds regression coverage for cache-hit behavior and asserts download_skipped on Parquet paths. |
tests/release/test_classification_release.py |
Aligns release fixtures with dataset label exclusions and per-split media roots for sharded datasets. |
opensportslib/setup/setup.py |
Normalizes detected CUDA version strings into PyTorch wheel tags before selecting the torch index URL. |
README.md |
Documents that existing per-split JSON outputs are reused for Parquet/WebDataset downloads. |
tests/release/README.md |
Documents the per-split JSON reuse behavior in release test tooling. |
Suppressed comments (1)
opensportslib/setup/setup.py:77
- When
CUDA_VERSIONis not detected (CPU-only machine),install_torch()enters thefor cuda in CUDA_SUPPORT:loop, installs plain CPU wheels, but then returns the first CUDA tag in the list (currentlycu126). That return value is misleading and can cause callers/logs to report the wrong install target.
# 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
subprocess.check_call([
python, "-m", "pip", "install",
"torch", "torchvision", "torchaudio",
"--index-url",
f"https://download.pytorch.org/whl/{cuda}"
])
print(f"\nSuccess with {cuda}")
return cuda
for cuda in CUDA_SUPPORT:
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
220
to
+245
| os.makedirs(output_dir, exist_ok=True) | ||
| 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, | ||
| }, | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
API impact
Backward-compatible: Parquet download results now include download_skipped. Existing converted JSON files are reused.