Repository navigation
Conversation
masquare
added a commit
that referenced
this pull request
Nov 10, 2026
… on the series Description The review of #615 asked for two more changes, and the plots dataset can now use TimeNet's own parent mechanism instead of subclassing: * Both connectors build on `BaseHuggingFaceConnector`, whose source parts can now be file patterns (#770). The series connector declares the question table and `arfbench-ts-data/*.parquet`; the plots connector the table and `arfbench-images/*.png`. * Three categories (presence, correlation, indicator: 444 of the 750 questions) offer the same options in every question. Each is now a label set, and their questions are classification tasks over it. The other questions offer their own options, such as timestamps or tag groups, and stay answer tasks with the `answer_options` annotation. A release whose options differ from a label set fails the conversion. * `datadog/arfbench-plots` declares `datadog/arfbench@1.0.0` as its parent `series`. Its `compose` imports the parent's tasks with the metric records they cite and adds the charts and the chart tasks. The imported series keep reading the parent's Parquet release, so the child stores only the charts, with Zarr. Test plan - Command: pytest and ty for both connectors in their own environments, as `make test-connectors` runs them - Coverage: 15 series tests (label-set and answer tasks, the Parquet round trip, a fixed category with other options) and 4 plots tests (the parent declaration, the imported and added tasks, a child that stores only the charts and reads the series through its parent, a missing chart) - Command: `make check` - Coverage: formatting, lint, generated schemas, and types pass - Command: `timenet-build build datadog/arfbench-plots --build-parents` into a local registry - Coverage: `datadog/arfbench` holds 142 records, 38,032 series, and 444 classification and 306 answer tasks; `datadog/arfbench-plots` holds 556 records and 1,500 tasks, names `datadog/arfbench@1.0.0` as its payload source, and stores 40 MB of charts
masquare
force-pushed
the
feature/hub-base-patterns
branch
from
November 10, 2026 15:52
9fbf821 to
0f96460
Compare
masquare
added a commit
that referenced
this pull request
Nov 10, 2026
…e the plots on the series Description The review of #615 asked for two more changes, and the plots dataset can now use TimeNet's own parent mechanism instead of subclassing: * Both connectors build on `BaseHuggingFaceConnector`, whose source parts can now be file patterns (#770). The series connector declares the question table and `arfbench-ts-data/*.parquet`; the plots connector the table and `arfbench-images/*.png`. * Every question asks to pick one of its released options, so each task is a classification task whose label set holds those options, named after the question's row. The series task and the chart task of a question share it. The `answer_options` annotation goes. * `datadog/arfbench-plots` declares `datadog/arfbench@1.0.0` as its parent `series`. Its `compose` imports the parent's tasks with the metric records they cite and adds the charts and the chart tasks. The imported series keep reading the parent's Parquet release, so the child stores only the charts, with Zarr. Test plan - Command: pytest and ty for both connectors in their own environments, as `make test-connectors` runs them - Coverage: 14 series tests (the classification tasks and their label sets, the Parquet round trip) and 4 plots tests (the parent declaration, the imported and added tasks, a child that stores only the charts and reads the series through its parent, a missing chart) - Command: `make check` - Coverage: formatting, lint, generated schemas, and types pass - Command: `timenet-build build datadog/arfbench-plots --build-parents` into a local registry - Coverage: `datadog/arfbench` holds 142 records, 38,032 series, and 750 classification tasks; `datadog/arfbench-plots` holds 556 records and 1,500 tasks over the same 750 label sets, names `datadog/arfbench@1.0.0` as its payload source, and stores the charts
masquare
marked this pull request as ready for review
November 10, 2026 15:55
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I checked for callers left behind by the removal of download_hub_files and the change to hub_snapshot's signature. Nothing still uses download_hub_files. The ARFBench connector (datasets/datadog/arfbench/connector.py:97) already calls hub_snapshot(repo, revision, cache_dir, patterns) with the new signature, so it isn't affected.
Extended reasoning...
A search of the repo found no remaining references to download_hub_files. The only callers of hub_snapshot are the base connector and ARFBench, and both pass the new four-argument signature.
Findings marked 🟡 are optional suggestions and need no follow-up push.
masquare
added a commit
that referenced
this pull request
Nov 10, 2026
…e the plots on the series Description The review of #615 asked for two more changes, and the plots dataset can now use TimeNet's own parent mechanism instead of subclassing: * Both connectors build on `BaseHuggingFaceConnector`, whose source parts can now be file patterns (#770). The series connector declares the question table and `arfbench-ts-data/*.parquet`; the plots connector the table and `arfbench-images/*.png`. * Every question asks to pick one of its released options, so each task is a classification task whose label set holds those options, named after the question's row. The series task and the chart task of a question share it. The `answer_options` annotation goes. * `datadog/arfbench-plots` declares `datadog/arfbench@1.0.0` as its parent `series`. Its `compose` imports the parent's tasks with the metric records they cite and adds the charts and the chart tasks. The imported series keep reading the parent's Parquet release, so the child stores only the charts, with Zarr. Test plan - Command: pytest and ty for both connectors in their own environments, as `make test-connectors` runs them - Coverage: 14 series tests (the classification tasks and their label sets, the Parquet round trip) and 4 plots tests (the parent declaration, the imported and added tasks, a child that stores only the charts and reads the series through its parent, a missing chart) - Command: `make check` - Coverage: formatting, lint, generated schemas, and types pass - Command: `timenet-build build datadog/arfbench-plots --build-parents` into a local registry - Coverage: `datadog/arfbench` holds 142 records, 38,032 series, and 750 classification tasks; `datadog/arfbench-plots` holds 556 records and 1,500 tasks over the same 750 label sets, names `datadog/arfbench@1.0.0` as its payload source, and stores the charts
There was a problem hiding this comment.
An organization admin can review the limit in your organization's usage and billing settings.
Once the limit resets or is raised, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
TimoStoff
reviewed
Nov 10, 2026
| """An immutable commit: a mutable branch can silently change rebuilt datasets.""" | ||
| SOURCE_FILES: ClassVar[Mapping[str, str]] | ||
| """Each part's repository-relative filename, in the order ``download()`` returns them.""" | ||
| """Each part's repository-relative filename, in the order ``download()`` returns them. A filename can end |
Contributor
There was a problem hiding this comment.
Still quite claudey. Better would be e.g
Suggested change
| """Each part's repository-relative filename, in the order ``download()`` returns them. A filename can end | |
| """The repo-relative filename of each part, in download-order. A filename can end |
…attern Description `BaseHuggingFaceConnector` downloads the files that `SOURCE_FILES` names, one filename per part. A release that holds many files of one kind, such as ARFBench with 748 series files and 414 charts, cannot list them, so its connector used `hub_snapshot` directly instead of the base. `hub_snapshot` is now the one Hub helper. It passes the patterns straight to `snapshot_download`, which matches globs itself, and returns the snapshot directory, so the listing call, the pattern matching, and `download_hub_files` go. The base downloads its parts with it, so a filename can end in a glob, such as `data/*.parquet`. That part's `HubFileRef` is the directory of the matching files. A part that matches no file fails with its name, as a missing file did before. Test plan - Command: `make check` - Coverage: formatting, lint, generated schemas, and types pass - Command: `uv run pytest packages/timenet-connectors/tests` - Coverage: 320 tests pass, including a pattern part and a pattern without a match, against a fake Hub that applies the patterns - Command: pytest for the tsqa, tsaqa_benchmark, sensorcaps, and arfbench connectors, which call the base or the helper - Coverage: their 81 tests pass unchanged
…English Description The docstrings that this PR adds now follow the rules of simplified technical English: one fact per sentence, sentences of 25 words or fewer, and no semicolons. No code changes. Test plan - Command: `make check` - Coverage: formatting, lint, generated schemas, and types pass - Command: `uv run pytest packages/timenet-connectors/tests` - Coverage: 320 tests pass
TimoStoff
force-pushed
the
feature/hub-base-patterns
branch
from
November 10, 2026 20:10
75a0e9a to
a3d5e5f
Compare
TimoStoff
pushed a commit
that referenced
this pull request
Nov 10, 2026
…e the plots on the series Description The review of #615 asked for two more changes, and the plots dataset can now use TimeNet's own parent mechanism instead of subclassing: * Both connectors build on `BaseHuggingFaceConnector`, whose source parts can now be file patterns (#770). The series connector declares the question table and `arfbench-ts-data/*.parquet`; the plots connector the table and `arfbench-images/*.png`. * Every question asks to pick one of its released options, so each task is a classification task whose label set holds those options, named after the question's row. The series task and the chart task of a question share it. The `answer_options` annotation goes. * `datadog/arfbench-plots` declares `datadog/arfbench@1.0.0` as its parent `series`. Its `compose` imports the parent's tasks with the metric records they cite and adds the charts and the chart tasks. The imported series keep reading the parent's Parquet release, so the child stores only the charts, with Zarr. Test plan - Command: pytest and ty for both connectors in their own environments, as `make test-connectors` runs them - Coverage: 14 series tests (the classification tasks and their label sets, the Parquet round trip) and 4 plots tests (the parent declaration, the imported and added tasks, a child that stores only the charts and reads the series through its parent, a missing chart) - Command: `make check` - Coverage: formatting, lint, generated schemas, and types pass - Command: `timenet-build build datadog/arfbench-plots --build-parents` into a local registry - Coverage: `datadog/arfbench` holds 142 records, 38,032 series, and 750 classification tasks; `datadog/arfbench-plots` holds 556 records and 1,500 tasks over the same 750 label sets, names `datadog/arfbench@1.0.0` as its payload source, and stores the charts
|
| Check | Status |
|---|---|
| Quick checks | ❌ Failed |
| Tests (Python 3.13) | ⏭️ Did not run |
| Tests (Python 3.11) | ⏭️ Did not run |
Full output is in the job logs. Reproduce locally:
make lint-fix # auto-fix
make check-ci # the hooks, plus ty on 3.13 and 3.11
make test-unit # the quick job's tests
make test # the full suiteUpdated for a3d5e5f · run #8370
This branch has not been deployed
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.
Description
BaseHuggingFaceConnectordownloads the files thatSOURCE_FILESnames, with one filename perpart. Some releases hold many files of one kind. For example, ARFBench holds 748 series files and
414 charts. Such a release cannot list its files in
SOURCE_FILES. Thus the ARFBench connectorused
hub_snapshotdirectly instead of the base.hub_snapshotis now the only Hub helper. It sends the patterns directly tosnapshot_download,which matches the patterns itself. It returns the snapshot directory. Thus this PR removes the
listing call, the pattern matching, and
download_hub_files. The base useshub_snapshottodownload its parts. Thus a filename can end in a pattern, for example
data/*.parquet. TheHubFileRefof that part is the directory of the matching files. If a part matches no file, thedownload fails with the name of the part. A missing file gave the same result before.
Test plan
make checkuv run pytest packages/timenet-connectors/testsuse a fake Hub that applies the patterns.
the base or the helper