Skip to content

Commit

Permalink
Change approach to kwargs merging
Browse files Browse the repository at this point in the history
Using Config is fine when merging dicts but it breaks if
a top-level kwarg is a Mapping but should not be merged.
This was being triggered when formatter constructors
switched to using dataId=DataCoordinate. Reorganize
the merging such that only keys that appear in both
kwargs are merged using Config.
  • Loading branch information
timj committed May 17, 2024
1 parent 071cf95 commit 2d69742
Showing 1 changed file with 19 additions and 4 deletions.
23 changes: 19 additions & 4 deletions python/lsst/daf/butler/mapping_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,27 @@ def getFromRegistryWithMatch(
# Supplied keyword args must overwrite registry defaults
# We want this overwriting to happen recursively since we expect
# some of these keyword arguments to be dicts.
# Simplest to use Config for this
# Simplest to use Config for this but have to be careful for top-level
# objects that look like Mappings but aren't (eg DataCoordinate) so
# only merge keys that appear in both kwargs.
merged_kwargs: dict[str, Any] = {}
if kwargs and registry_kwargs:
config_kwargs = Config(registry_kwargs)
config_kwargs.update(kwargs)
merged_kwargs = config_kwargs.toDict()
kwargs_keys = set(kwargs.keys())
registry_keys = set(registry_kwargs.keys())
common_keys = kwargs_keys & registry_keys
if common_keys:
config_kwargs = Config({k: registry_kwargs[k] for k in common_keys})
config_kwargs.update({k: kwargs[k] for k in common_keys})
merged_kwargs = config_kwargs.toDict()

if kwargs_only := kwargs_keys - common_keys:
merged_kwargs.update({k: kwargs[k] for k in kwargs_only})
if registry_only := registry_keys - common_keys:
merged_kwargs.update({k: registry_kwargs[k] for k in registry_only})
else:
# Distinct in each.
merged_kwargs = kwargs
merged_kwargs.update(registry_kwargs)
elif registry_kwargs:
merged_kwargs = registry_kwargs
elif kwargs:
Expand Down

0 comments on commit 2d69742

Please sign in to comment.