Skip to content

Commit 55cc71c

Browse files
committed
refactor(config): dedup resolver helpers and fix deferred review items (refs #301)
1 parent 0c2e540 commit 55cc71c

11 files changed

Lines changed: 93 additions & 45 deletions

File tree

docs/using-raitap/configuration/python-api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ run(cfg, auto_install_deps=True)
8080

8181
| YAML pattern | Python builder |
8282
| ---------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
83-
| `use: captum` + `algorithm: IntegratedGradients` | `captum(algorithm="IntegratedGradients", ...)` (the `use` key is baked in) |
83+
| `use: captum` + `algorithm: IntegratedGradients` | `captum(algorithm="IntegratedGradients", ...)` (the `use` key is baked in) |
8484
| `defaults: [raitap_schema, _self_]` | Not needed — `AppConfig` already *is* the schema. The defaults entry is a Hydra-only construct. |
8585
| Group/name selection (`transparency: captum` + dict key in YAML) | Use the dict key on the Python side too: `transparency={"my_run": captum(algorithm=...)}`. |
8686
| List of visualisers | One builder per visualiser (`captum_image`, `image_pair`, …): flat constructor kwargs, optional `call={...}` for render-time options. `visualisers=[captum_image(max_samples=4, call={"show_sample_names": True})]`. |

src/raitap/_adapters.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ def __call__(self, ctx: CtxT, /) -> ResultT: ...
8787
# (:func:`raitap.configs.registry_resolve.resolve_target_fqn`). Group
8888
# ``"_unscoped"`` holds visualisers, which have no Hydra config group.
8989
_TARGET_FQN: dict[str, dict[str, str]] = {}
90+
# group -> FamilyConfig.package_style ("nested" | "flat"). The source of truth
91+
# for whether a group's Hydra config holds multiple named entries
92+
# (``cfg.<group>.<name>``) or a single one (``cfg.<group>``) — read by
93+
# :mod:`raitap._config_schema` instead of hardcoding the group set there.
94+
_GROUP_PACKAGE_STYLE: dict[str, str] = {}
9095
# adapter class name -> uv extra (consumed by raitap.deps.inference)
9196
ADAPTER_EXTRAS: dict[str, str] = {}
9297
# group -> set of wrapped third-party library names; used by
@@ -357,6 +362,7 @@ def _register_core(
357362
cls._adapter_group = family.group
358363
fqn = _class_fqn(cls)
359364
_TARGET_FQN.setdefault(family.group, {})[registry_name] = fqn
365+
_GROUP_PACKAGE_STYLE[family.group] = family.package_style
360366
schema = schema_override or family.schema
361367
builder = _use_node(schema, registry_name, cls.__name__)
362368
# Hydra groups use ``/`` for nesting; OmegaConf packages use ``.``.

src/raitap/_config_schema.py

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,30 @@
3333
if TYPE_CHECKING:
3434
from collections.abc import Iterable
3535

36-
# Families whose Hydra package style is "nested" (:class:`raitap._adapters.
37-
# FamilyConfig`, ``package_style="nested"``) — multiple named entries share
38-
# the group. Every other group (including ``data/labels`` / ``data/inputs``)
39-
# is "flat": one config per group.
40-
_NESTED_GROUPS = frozenset({"transparency", "robustness"})
36+
# Fallback only: used when a group is missing from
37+
# :data:`raitap._adapters._GROUP_PACKAGE_STYLE` (e.g. no adapter of that
38+
# family has registered yet, or the "_unscoped"/data groups, which carry no
39+
# ``FamilyConfig``). The live map is the source of truth — see
40+
# :func:`_group_is_nested`.
41+
_NESTED_GROUPS_FALLBACK = frozenset({"transparency", "robustness"})
42+
43+
44+
def _group_is_nested(group: str) -> bool:
45+
"""True when ``group``'s Hydra package style is "nested" (multiple named
46+
entries share the group, e.g. ``cfg.transparency.<name>.use``).
47+
48+
Reads :data:`raitap._adapters._GROUP_PACKAGE_STYLE`, populated by each
49+
family decorator from its own :class:`~raitap._adapters.FamilyConfig`
50+
(the real source of truth), falling back to the historical hardcoded set
51+
only when the group isn't recorded there (e.g. ``"_unscoped"``, ``data/*``,
52+
or a family with no adapters registered yet).
53+
"""
54+
from raitap._adapters import _GROUP_PACKAGE_STYLE
55+
56+
style = _GROUP_PACKAGE_STYLE.get(group)
57+
if style is not None:
58+
return style == "nested"
59+
return group in _NESTED_GROUPS_FALLBACK
4160

4261

4362
def _use_enum_schema(registry_names: Iterable[str]) -> dict[str, Any]:
@@ -92,7 +111,7 @@ def build_config_schema() -> dict[str, Any]:
92111
if group == "_unscoped":
93112
properties["visualiser"] = _group_schema(registry_names, nested=False)
94113
continue
95-
schema = _group_schema(registry_names, nested=group in _NESTED_GROUPS)
114+
schema = _group_schema(registry_names, nested=_group_is_nested(group))
96115
_set_nested_property(properties, group.split("/"), schema)
97116

98117
return {

src/raitap/configs/registry_resolve.py

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,24 @@ def reject_config_target(cfg: Any) -> None:
3939
reject_config_target(item)
4040

4141

42+
def use_key_enabled(cfg: Any) -> bool:
43+
"""True when ``cfg`` (already ``cfg_to_dict``'d) carries a non-empty ``use``.
44+
45+
Shared by :func:`raitap.metrics.factory.metrics_run_enabled` and
46+
:func:`raitap.reporting.factory.reporting_enabled`, which both gate an
47+
optional config block on "is `use` set to a non-empty string", after
48+
rejecting a ``_target_``-carrying block loudly instead of silently reading
49+
it as "not configured". Callers keep their own ``None``-config
50+
short-circuit (this helper assumes a non-``None`` config was already
51+
turned into a dict).
52+
"""
53+
reject_config_target(cfg)
54+
use = cfg.get("use")
55+
if use is None:
56+
return False
57+
return bool(str(use).strip())
58+
59+
4260
def resolve_target_fqn(group: str, use: str) -> str:
4361
from raitap._adapters import _TARGET_FQN
4462

@@ -47,11 +65,32 @@ def resolve_target_fqn(group: str, use: str) -> str:
4765
return group_map[use]
4866
except KeyError:
4967
valid = ", ".join(sorted(group_map))
68+
error_group = "visualiser" if group == "_unscoped" else group
5069
raise ValueError(
51-
f"Unknown {group} key {use!r}. Valid keys: {valid or '(none registered)'}."
70+
f"Unknown {error_group} key {use!r}. Valid keys: {valid or '(none registered)'}."
5271
) from None
5372

5473

74+
def stamp_target_from_use(cfg: dict[str, Any], *, group: str) -> None:
75+
"""Reject ``_target_``, resolve ``cfg["use"]`` against ``group``, and stamp
76+
the resolved FQN back onto ``cfg`` as ``_target_`` (popping ``use``).
77+
78+
Mutates ``cfg`` in place. Shared by
79+
:func:`raitap.metrics.factory.create_metric`,
80+
:func:`raitap.transparency.evaluation.step.grade_explanations`, and
81+
:func:`raitap.data._parser_factory.create_parser`, which all resolve a
82+
``use:``-keyed config to a vetted ``_target_`` before handing it to
83+
``hydra.utils.instantiate``. Callers keep their own post-instantiate
84+
tails (return shape, error wording) — this helper only covers the shared
85+
reject-resolve-stamp-pop core. Preserves the security ordering: ``cfg``
86+
is rejected for a smuggled ``_target_`` *before* ``use`` is resolved.
87+
"""
88+
reject_config_target(cfg)
89+
use = str(cfg.get("use", ""))
90+
cfg["_target_"] = resolve_target_fqn(group, use)
91+
cfg.pop("use", None)
92+
93+
5594
def instantiate_partial_from_use(
5695
cfg: Any,
5796
*,

src/raitap/data/_parser_factory.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,15 @@
88

99
from raitap import raitap_log
1010
from raitap.configs import cfg_to_dict
11-
from raitap.configs.registry_resolve import reject_config_target, resolve_target_fqn
11+
from raitap.configs.registry_resolve import stamp_target_from_use
1212

1313

1414
def create_parser(config: Any, *, group: str, kind: str) -> Any:
1515
"""Instantiate a parser from a ``use:``-keyed config, resolved via the
1616
trusted registry seam (``raitap.configs.registry_resolve``)."""
1717
cfg = cfg_to_dict(config)
18-
reject_config_target(cfg)
19-
use = cfg.get("use", "")
20-
fqn = resolve_target_fqn(group, use)
21-
cfg["_target_"] = fqn
22-
cfg.pop("use", None)
18+
stamp_target_from_use(cfg, group=group)
19+
fqn = cfg["_target_"]
2320
try:
2421
return instantiate(cfg)
2522
except Exception as e:

src/raitap/deps/static_scan.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,10 @@ def scan_adapter_registry() -> dict[str, dict[str, str]]:
191191
the legacy ``_target_`` lookup. This scanner mirrors that: the family
192192
decorator name (e.g. ``robustness_adapter``) determines the group via
193193
:data:`_DECORATOR_GROUP`, and ``extra`` defaults to ``registry_name`` the
194-
same way :func:`scan_adapter_extras` and ``_register_core`` do.
194+
same way :func:`scan_adapter_extras` and ``_register_core`` do — except for
195+
the ``"_unscoped"`` group (``transparency_evaluator``, ``family=None`` at
196+
runtime), where ``_register_core`` gives no auto-extra at all: only an
197+
explicit ``extra=`` kwarg counts, else the extra is left empty.
195198
"""
196199
import raitap
197200

@@ -203,8 +206,10 @@ def scan_adapter_registry() -> dict[str, dict[str, str]]:
203206
continue
204207
kwargs = _string_kwargs(deco)
205208
registry_name = kwargs.get("registry_name")
206-
if registry_name:
207-
found.setdefault(group, {})[registry_name] = kwargs.get("extra", registry_name)
209+
if not registry_name:
210+
continue
211+
default_extra = "" if group == "_unscoped" else registry_name
212+
found.setdefault(group, {})[registry_name] = kwargs.get("extra", default_extra)
208213
return found
209214

210215

src/raitap/metrics/factory.py

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
from raitap import raitap_log
1313
from raitap.configs import cfg_to_dict, resolve_run_dir
14-
from raitap.configs.registry_resolve import reject_config_target, resolve_target_fqn
14+
from raitap.configs.registry_resolve import stamp_target_from_use, use_key_enabled
1515
from raitap.reporting.sections import Reportable, ReportGroup, ReportSection
1616
from raitap.reporting.staging import _copy_asset
1717
from raitap.tracking.base_tracker import BaseTracker, Trackable
@@ -34,24 +34,15 @@ def metrics_run_enabled(config: AppConfig) -> bool:
3434
metrics_cfg = config.metrics
3535
if metrics_cfg is None:
3636
return False
37-
metrics_dict = cfg_to_dict(metrics_cfg)
38-
# Reject a `_target_`-carrying block loudly instead of silently reading it
39-
# as "not configured" — this guard gates non-schema-checked config blocks.
40-
reject_config_target(metrics_dict)
41-
use = metrics_dict.get("use")
42-
if use is None:
43-
return False
44-
return bool(str(use).strip())
37+
return use_key_enabled(cfg_to_dict(metrics_cfg))
4538

4639

4740
def create_metric(metrics_config: Any) -> tuple[BaseMetricComputer, str]:
4841
"""Instantiate a metric computer from config (``use: <registry key>`` + kwargs)."""
4942
metrics_cfg = cfg_to_dict(metrics_config)
50-
reject_config_target(metrics_cfg)
5143
use = str(metrics_cfg.get("use", ""))
52-
resolved_target = resolve_target_fqn("metrics", use)
53-
metrics_cfg["_target_"] = resolved_target
54-
metrics_cfg.pop("use", None)
44+
stamp_target_from_use(metrics_cfg, group="metrics")
45+
resolved_target = metrics_cfg["_target_"]
5546

5647
try:
5748
metric = instantiate(metrics_cfg)

src/raitap/reporting/factory.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
from raitap import raitap_log
1010
from raitap.configs import cfg_to_dict
11-
from raitap.configs.registry_resolve import instantiate_partial_from_use, reject_config_target
11+
from raitap.configs.registry_resolve import instantiate_partial_from_use, use_key_enabled
1212
from raitap.tracking.base_tracker import BaseTracker, Trackable
1313

1414
if TYPE_CHECKING:
@@ -24,14 +24,7 @@ def reporting_enabled(config: AppConfig) -> bool:
2424
"""Check if reporting is enabled in config."""
2525
if config.reporting is None:
2626
return False
27-
reporting_cfg = cfg_to_dict(config.reporting)
28-
# Reject a `_target_`-carrying block loudly instead of silently reading it
29-
# as "disabled" — this guard gates non-schema-checked callback configs.
30-
reject_config_target(reporting_cfg)
31-
use = reporting_cfg.get("use")
32-
if use is None:
33-
return False
34-
return bool(str(use).strip())
27+
return use_key_enabled(cfg_to_dict(config.reporting))
3528

3629

3730
@dataclass

src/raitap/tracking/smoke_test_mlflow.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ def main() -> int:
104104
num_classes=1000,
105105
),
106106
tracking=TrackingConfig(
107+
use="mlflow",
107108
output_forwarding_url=tracking_uri,
108109
log_model=args.log_model,
109110
),

src/raitap/transparency/evaluation/step.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
from hydra.utils import instantiate
1818

19-
from raitap.configs.registry_resolve import reject_config_target, resolve_target_fqn
19+
from raitap.configs.registry_resolve import stamp_target_from_use
2020
from raitap.configs.utils import cfg_to_dict
2121
from raitap.transparency.evaluation.semantics import EvaluationContext
2222

@@ -42,10 +42,7 @@ def grade_explanations(
4242
if evaluation_cfg is None or not explanations:
4343
return []
4444
cfg = cfg_to_dict(evaluation_cfg)
45-
reject_config_target(cfg)
46-
use = cfg.get("use", "")
47-
cfg["_target_"] = resolve_target_fqn("_unscoped", use)
48-
cfg.pop("use", None)
45+
stamp_target_from_use(cfg, group="_unscoped")
4946
evaluator = instantiate(cfg)
5047
backend = prepared.backend # type: ignore[attr-defined]
5148
model = backend.autograd_module()

0 commit comments

Comments
 (0)