Skip to content

Commit 695207e

Browse files
committed
feat(config): unify dataset source via discriminated source: block
Replace the parallel gitlab_identifier / huggingface_identifier fields on DatasetConfig with a single self-describing source: block (type selects the backend). Legacy fields keep working: a legacy identifier is mirrored into source (with a DeprecationWarning), and a source: is back-filled into the matching legacy field so existing consumers that read the *_identifier fields are unaffected. Specifying both is rejected. Addresses FEP-1025 (reduce configuration friction, epic #1205). Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
1 parent 25f31d0 commit 695207e

3 files changed

Lines changed: 193 additions & 1 deletion

File tree

nemo_gym/config_types.py

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
15+
import warnings
1516
from argparse import ArgumentParser
1617
from enum import Enum
1718
from pathlib import Path
18-
from typing import Any, ClassVar, Dict, List, Literal, Optional, Set, Tuple, Union
19+
from typing import Annotated, Any, ClassVar, Dict, List, Literal, Optional, Set, Tuple, Union
1920

2021
import rich
2122
from omegaconf import DictConfig, OmegaConf
@@ -358,12 +359,37 @@ def check_output_path(self) -> "DownloadJsonlDatasetHuggingFaceConfig":
358359
DatasetType = Union[Literal["train"], Literal["validation"], Literal["example"]]
359360

360361

362+
class GitlabDatasetSource(BaseModel):
363+
"""Unified ``source:`` for a dataset fetched from the GitLab model registry."""
364+
365+
type: Literal["gitlab"]
366+
dataset_name: str
367+
version: str
368+
artifact_fpath: str
369+
370+
371+
class HuggingFaceDatasetSource(BaseModel):
372+
"""Unified ``source:`` for a dataset fetched from the HuggingFace Hub."""
373+
374+
type: Literal["huggingface"]
375+
repo_id: str
376+
artifact_fpath: Optional[str] = None
377+
378+
379+
# One discriminated `source:` block replaces the parallel gitlab_identifier / huggingface_identifier
380+
# fields; `type` selects the backend so it's unambiguous which fields apply.
381+
DatasetSource = Annotated[Union[GitlabDatasetSource, HuggingFaceDatasetSource], Field(discriminator="type")]
382+
383+
361384
class DatasetConfig(BaseModel):
362385
name: str
363386
type: DatasetType
364387
jsonl_fpath: str
365388

366389
num_repeats: int = Field(default=1, ge=1)
390+
# Unified, self-describing dataset source. Prefer this over the legacy *_identifier fields below.
391+
source: Optional[DatasetSource] = None
392+
# Deprecated: kept working (and back-filled from/into `source`) for backward compatibility.
367393
gitlab_identifier: Optional[JsonlDatasetGitlabIdentifer] = None
368394
huggingface_identifier: Optional[JsonlDatasetHuggingFaceIdentifer] = None
369395
license: Optional[
@@ -386,6 +412,54 @@ def check_train_validation_sets(self) -> "DatasetConfig":
386412

387413
return self
388414

415+
@model_validator(mode="after")
416+
def normalize_dataset_source(self) -> "DatasetConfig":
417+
"""Reconcile the unified `source:` with the legacy `*_identifier` fields.
418+
419+
Exactly one source may be specified. A legacy identifier is accepted (with a deprecation
420+
warning) and mirrored into `source`; conversely a `source:` is mirrored back into the
421+
matching legacy field so existing consumers that read `gitlab_identifier`/
422+
`huggingface_identifier` keep working.
423+
"""
424+
specified = [
425+
name
426+
for name, value in (
427+
("source", self.source),
428+
("gitlab_identifier", self.gitlab_identifier),
429+
("huggingface_identifier", self.huggingface_identifier),
430+
)
431+
if value is not None
432+
]
433+
if len(specified) > 1:
434+
raise ValueError(
435+
f"Specify a dataset source once for '{self.name}': set only one of {specified}. "
436+
"Prefer the unified `source:` block."
437+
)
438+
if not specified:
439+
return self
440+
441+
if self.source is None:
442+
# Legacy identifier was used: mirror it into `source` and nudge toward the new field.
443+
if self.gitlab_identifier is not None:
444+
self.source = GitlabDatasetSource(type="gitlab", **self.gitlab_identifier.model_dump())
445+
else:
446+
self.source = HuggingFaceDatasetSource(type="huggingface", **self.huggingface_identifier.model_dump())
447+
warnings.warn(
448+
f"`{specified[0]}` is deprecated for dataset '{self.name}'; use the unified "
449+
f"`source:` block (type: {self.source.type}).",
450+
DeprecationWarning,
451+
stacklevel=2,
452+
)
453+
else:
454+
# `source:` was used: back-fill the matching legacy field for existing consumers.
455+
fields = self.source.model_dump(exclude={"type"})
456+
if isinstance(self.source, GitlabDatasetSource):
457+
self.gitlab_identifier = JsonlDatasetGitlabIdentifer(**fields)
458+
else:
459+
self.huggingface_identifier = JsonlDatasetHuggingFaceIdentifer(**fields)
460+
461+
return self
462+
389463

390464
class BenchmarkDatasetConfig(BaseModel):
391465
name: str
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
from pydantic import ValidationError
16+
from pytest import raises, warns
17+
18+
from nemo_gym.config_types import (
19+
DatasetConfig,
20+
GitlabDatasetSource,
21+
HuggingFaceDatasetSource,
22+
)
23+
24+
25+
def _dataset(**extra) -> dict:
26+
return {"name": "ds", "type": "example", "jsonl_fpath": "data.jsonl", **extra}
27+
28+
29+
class TestDatasetSource:
30+
def test_source_gitlab_backfills_legacy_identifier(self) -> None:
31+
cfg = DatasetConfig.model_validate(
32+
_dataset(
33+
source={
34+
"type": "gitlab",
35+
"dataset_name": "my_dataset",
36+
"version": "0.0.1",
37+
"artifact_fpath": "train.jsonl",
38+
}
39+
)
40+
)
41+
42+
assert isinstance(cfg.source, GitlabDatasetSource)
43+
# Existing consumers read the legacy field; it must be back-filled from `source`.
44+
assert cfg.gitlab_identifier is not None
45+
assert cfg.gitlab_identifier.dataset_name == "my_dataset"
46+
assert cfg.gitlab_identifier.version == "0.0.1"
47+
assert cfg.gitlab_identifier.artifact_fpath == "train.jsonl"
48+
assert cfg.huggingface_identifier is None
49+
50+
def test_source_huggingface_backfills_legacy_identifier(self) -> None:
51+
cfg = DatasetConfig.model_validate(
52+
_dataset(source={"type": "huggingface", "repo_id": "org/dataset", "artifact_fpath": "train.jsonl"})
53+
)
54+
55+
assert isinstance(cfg.source, HuggingFaceDatasetSource)
56+
assert cfg.huggingface_identifier is not None
57+
assert cfg.huggingface_identifier.repo_id == "org/dataset"
58+
assert cfg.huggingface_identifier.artifact_fpath == "train.jsonl"
59+
assert cfg.gitlab_identifier is None
60+
61+
def test_legacy_gitlab_identifier_mirrors_into_source_with_warning(self) -> None:
62+
with warns(DeprecationWarning, match="gitlab_identifier"):
63+
cfg = DatasetConfig.model_validate(
64+
_dataset(
65+
gitlab_identifier={
66+
"dataset_name": "my_dataset",
67+
"version": "0.0.1",
68+
"artifact_fpath": "train.jsonl",
69+
}
70+
)
71+
)
72+
73+
assert isinstance(cfg.source, GitlabDatasetSource)
74+
assert cfg.source.dataset_name == "my_dataset"
75+
assert cfg.source.version == "0.0.1"
76+
assert cfg.source.artifact_fpath == "train.jsonl"
77+
# Legacy field stays populated so nothing that already reads it breaks.
78+
assert cfg.gitlab_identifier is not None
79+
80+
def test_legacy_huggingface_identifier_mirrors_into_source_with_warning(self) -> None:
81+
with warns(DeprecationWarning, match="huggingface_identifier"):
82+
cfg = DatasetConfig.model_validate(_dataset(huggingface_identifier={"repo_id": "org/dataset"}))
83+
84+
assert isinstance(cfg.source, HuggingFaceDatasetSource)
85+
assert cfg.source.repo_id == "org/dataset"
86+
assert cfg.source.artifact_fpath is None
87+
assert cfg.huggingface_identifier is not None
88+
89+
def test_specifying_source_and_legacy_identifier_is_rejected(self) -> None:
90+
with raises(ValidationError, match="set only one"):
91+
DatasetConfig.model_validate(
92+
_dataset(
93+
source={
94+
"type": "gitlab",
95+
"dataset_name": "my_dataset",
96+
"version": "0.0.1",
97+
"artifact_fpath": "train.jsonl",
98+
},
99+
gitlab_identifier={
100+
"dataset_name": "my_dataset",
101+
"version": "0.0.1",
102+
"artifact_fpath": "train.jsonl",
103+
},
104+
)
105+
)
106+
107+
def test_no_source_is_allowed(self) -> None:
108+
cfg = DatasetConfig.model_validate(_dataset())
109+
110+
assert cfg.source is None
111+
assert cfg.gitlab_identifier is None
112+
assert cfg.huggingface_identifier is None
113+
114+
def test_source_discriminator_selects_backend(self) -> None:
115+
with raises(ValidationError):
116+
# Missing repo_id for the huggingface branch.
117+
DatasetConfig.model_validate(_dataset(source={"type": "huggingface", "artifact_fpath": "train.jsonl"}))

tests/unit_tests/test_train_data_utils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ def test_load_and_validate_server_instance_configs_sanity(self, monkeypatch: Mon
125125
"type": "example",
126126
"jsonl_fpath": "resources_servers/example_multi_step/data/example.jsonl",
127127
"num_repeats": 1,
128+
"source": None,
128129
"gitlab_identifier": None,
129130
"huggingface_identifier": None,
130131
"license": None,

0 commit comments

Comments
 (0)