Skip to content

Commit 0b44672

Browse files
committed
improve error messages for common config mistakes
This covers the main confusing cases: missing required key, bad module paths, and malformed CLI overrides like `key` without `=`. Unused config keys now produce direct JaQMC errors instead of a generic stop or mixed raw exceptions.
1 parent 6dee554 commit 0b44672

9 files changed

Lines changed: 464 additions & 89 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ dependencies = [
3636
"jax>=0.4.36,<=0.9.1",
3737
"kfac-jax==0.0.8",
3838
"numpy>=2.3.5",
39-
"pyserde>=0.31.1",
39+
"pyserde>=0.31.7",
4040
"optax>=0.2.5",
4141
"pygments>=2.19.2",
4242
"pyscf>=2.11.0",

requirements.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ pygments==2.20.0
181181
# rich
182182
pyscf==2.12.1
183183
# via jaqmc
184-
pyserde==0.31.2
184+
pyserde==0.31.7
185185
# via jaqmc
186186
pytest==9.0.3
187187
# via

src/jaqmc/utils/cli.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import click
77
import yaml
88

9-
from jaqmc.utils.config import ConfigManager
9+
from jaqmc.utils.config import ConfigError, ConfigManager
1010
from jaqmc.utils.runtime import configure_runtime
1111

1212

@@ -34,9 +34,12 @@ def make_cli(workflow=None, **kwargs):
3434
def command(
3535
dotlist: tuple[str, ...], yml: Sequence[str], dry_run: bool = False
3636
) -> None:
37-
cfg = ConfigManager([load_yaml(f) for f in yml], list(dotlist))
38-
configure_runtime(cfg, dry_run=dry_run)
39-
workflow(cfg, dry_run=dry_run)
37+
try:
38+
cfg = ConfigManager([load_yaml(f) for f in yml], list(dotlist))
39+
configure_runtime(cfg, dry_run=dry_run)
40+
workflow(cfg, dry_run=dry_run)
41+
except ConfigError as e:
42+
raise click.ClickException(str(e)) from None
4043

4144
return command
4245

@@ -51,11 +54,14 @@ def load_yaml(f):
5154
Plain dictionary config.
5255
5356
Raises:
54-
ValueError: YAML contains list config.
57+
ConfigError: YAML root is not a mapping.
5558
"""
59+
source = getattr(f, "name", "<stream>")
5660
config = yaml.safe_load(f)
5761
if not isinstance(config, dict):
58-
raise ValueError(
59-
f"JaQMC expects a dictionary config from {f.read()}. Got {type(config)}."
62+
got = "empty document" if config is None else type(config).__name__
63+
raise ConfigError(
64+
f"Invalid YAML config in '{source}': expected a mapping at the "
65+
f"document root, got {got}."
6066
)
6167
return config

src/jaqmc/utils/config.py

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from pygments.lexers import DiffLexer, YamlLexer
1818
from serde.core import field as serde_field
1919

20-
from jaqmc.utils.module_resolver import resolve_object
20+
from jaqmc.utils.module_resolver import ModuleResolutionError, resolve_object
2121
from jaqmc.utils.wiring import wire
2222
from jaqmc.utils.yaml_format import annotate_yaml_with_sources, dump_yaml
2323

@@ -27,6 +27,11 @@
2727
logging.getLogger(__name__), extra={"category": "config"}
2828
)
2929

30+
31+
class ConfigError(ValueError):
32+
"""Raised when JaQMC configuration is invalid."""
33+
34+
3035
# =============================================================================
3136
# Public API: Decorators and field helpers
3237
# =============================================================================
@@ -412,14 +417,14 @@ def finalize(
412417
any unused configuration keys.
413418
414419
Args:
415-
raise_on_unused: If True, raises `SystemExit` if there are any
420+
raise_on_unused: If True, raises :class:`ConfigError` if there are any
416421
unused configuration keys in the input.
417422
verbose: If True, includes docstrings and source information in
418423
the logged YAML output.
419424
compare_yaml: YAML from previous from to log the difference.
420425
421426
Raises:
422-
SystemExit: If `raise_on_unused` is True and unused keys are found.
427+
ConfigError: If `raise_on_unused` is True and unused keys are found.
423428
"""
424429
yaml_content = self.to_yaml(verbose=verbose)
425430
logger.info(
@@ -430,22 +435,28 @@ def finalize(
430435
)
431436
unused_yaml = _find_unused_yaml_path(self._visited_paths, self._user_config)
432437
unused_cli = _find_unused_yaml_path(self._visited_paths, self._cli_config)
433-
if unused_yaml:
438+
if (unused_yaml or unused_cli) and raise_on_unused:
439+
lines = ["Unused config keys detected:"]
440+
if unused_yaml:
441+
lines.append(f"- from YAML/API: {unused_yaml}")
442+
if unused_cli:
443+
lines.append(f"- from CLI: {unused_cli}")
444+
lines.append(
445+
"\nRemove these keys, or set `workflow.config.ignore_extra=true` to "
446+
"ignore extra config. If you call `cfg.finalize()` manually, use "
447+
"`raise_on_unused=False`."
448+
)
449+
raise ConfigError("\n".join(lines))
450+
elif unused_yaml:
434451
logger.warning(
435452
"The following configs are specified via YAML/API but not used: %s",
436453
sorted(unused_yaml),
437454
)
438-
if unused_cli:
455+
elif unused_cli:
439456
logger.warning(
440457
"The following configs are specified via CLI but not used: %s",
441458
sorted(unused_cli),
442459
)
443-
if (unused_yaml or unused_cli) and raise_on_unused:
444-
raise SystemExit(
445-
"Stopping due to invalid configs specified. Please consider using "
446-
"`raise_on_unused=False` if you are calling `cfg.finalize` manually, "
447-
"or pass workflow.config.ignore_extra=True if you are using CLI."
448-
)
449460
if compare_yaml is not None:
450461
diff = "\n".join(
451462
difflib.unified_diff(
@@ -492,7 +503,9 @@ def get_module(self, name: str, default_module: str | Callable | type = ""):
492503
else default_module[: default_module.rfind(".")]
493504
)
494505
module_name = self._get_primitive(f"{name}.module", default_module)
495-
make_module = resolve_object(module_name, package=module_base)
506+
make_module = self._resolve_config_module(
507+
f"{name}.module", module_name, package=module_base
508+
)
496509
else:
497510
default_module_name = (
498511
f"{default_module.__module__}:{default_module.__name__}"
@@ -504,7 +517,9 @@ def get_module(self, name: str, default_module: str | Callable | type = ""):
504517
)
505518
module_name = self._get_primitive(f"{name}.module", default_module_name)
506519
if module_name != default_module_name:
507-
make_module = resolve_object(module_name, package=module_base)
520+
make_module = self._resolve_config_module(
521+
f"{name}.module", module_name, package=module_base
522+
)
508523
else:
509524
make_module = default_module
510525
if is_dataclass(make_module):
@@ -530,8 +545,15 @@ def get_collection(
530545
for k, v in module_configs.items():
531546
if v is None:
532547
continue
548+
if not isinstance(v, dict):
549+
raise ConfigError(
550+
f"Invalid config at '{name}.{k}': expected a mapping, "
551+
f"got {type(v).__name__}."
552+
)
553+
if "module" not in v:
554+
raise ConfigError(f"Missing required config key '{name}.{k}.module'.")
533555
module_path = v["module"]
534-
make_module = resolve_object(module_path)
556+
make_module = self._resolve_config_module(f"{name}.{k}.module", module_path)
535557
if is_dataclass(make_module):
536558
default_item = default_dict.get(k, {})
537559
default_config = {
@@ -635,6 +657,11 @@ def _get_dataclass[DataclassT](
635657
base_config = serde.to_dict(default) if not inspect.isclass(default) else {}
636658

637659
config_data = user_config if user_config is not _MISSING else {}
660+
if user_config is not _MISSING and not isinstance(config_data, dict):
661+
raise ConfigError(
662+
f"Invalid config at '{name}': expected a mapping, "
663+
f"got {type(config_data).__name__}."
664+
)
638665
if isinstance(config_data, dict):
639666
config_data = {k: v for k, v in config_data.items() if k != "module"}
640667
else:
@@ -644,7 +671,7 @@ def _get_dataclass[DataclassT](
644671
try:
645672
result = serde.from_dict(cls, merged)
646673
except serde.SerdeError as e:
647-
raise serde.SerdeError(f"Invalid config at '{name}': {e}") from None
674+
raise ConfigError(f"Invalid config at '{name}': {e}") from None
648675

649676
if not inspect.isclass(default):
650677
_copy_runtime_fields(default, result)
@@ -656,6 +683,18 @@ def _get_dataclass[DataclassT](
656683

657684
return cast(DataclassT, result)
658685

686+
def _resolve_config_module(
687+
self,
688+
path: str,
689+
module_name: str,
690+
*,
691+
package: str | None = None,
692+
) -> Any:
693+
try:
694+
return resolve_object(module_name, package=package)
695+
except ModuleResolutionError as e:
696+
raise ConfigError(f"Invalid config at '{path}': {e}") from None
697+
659698
def _get_callable[CallableT: Callable](
660699
self,
661700
name: str,
@@ -717,7 +756,13 @@ def _get_path(data: dict[str, Any], path: list[str], default: Any = None) -> Any
717756
def _dotlist_to_dict(dotlist: list[str]) -> dict[str, Any]:
718757
result: dict[str, Any] = {}
719758
for entry in dotlist:
759+
if "=" not in entry:
760+
raise ConfigError(
761+
f"Invalid CLI override '{entry}': expected the form key=value."
762+
)
720763
key, _, raw_value = entry.partition("=")
764+
if not key:
765+
raise ConfigError(f"Invalid CLI override '{entry}': key must not be empty.")
721766
_set_path(result, key, yaml.safe_load(raw_value))
722767
return result
723768

0 commit comments

Comments
 (0)