-
Notifications
You must be signed in to change notification settings - Fork 45
Description
Describe the bug
isinstance(config_dict_obj, collections.abc.Mapping)
returns False
even though it is a "dictionary" type
This is because class ml_collections.config_dict.ConfigDict
does not inherit Python built-in abstract base class collections.abc.Mapping
, despite being a "dictionary" type, producing unexpected behavior when using reflection or duck-typing with isinstance(x, collections.abc.Mapping)
. This is especially problematic for serializing and logging code where it is common to apply different formatting to different collection types.
See PR #46 for proposed solution.
To Reproduce
The following code snippet reproduces and illustrates the bug
import collections.abc
from typing import Any
from ml_collections import config_dict
def safe_print(obj: Any):
"""Print object with masked values to avoid leaking API keys in the logs"""
if isinstance(obj, collections.abc.Mapping):
for k, v in obj.items():
print(f"{k}={'*' * len(str(v))}")
elif isinstance(obj, collections.abc.Sequence):
print(f"[ ... ({len(obj)} items)]")
else:
raise ValueError(f"Unsupported type: {type(obj)}")
safe_print({"a": 1})
>>> a=*
conf = config_dict.ConfigDict({"a": 1})
safe_print(conf)
>>> Traceback (most recent call last):
>>> File "<stdin>", line 1, in <module>
>>> File "<stdin>", line 8, in safe_print
>>> ValueError: Unsupported type: <class 'ml_collections.config_dict.config_dict.ConfigDict'>
Expected behavior
isinstance(config_dict_obj, collections.abc.Mapping)
should return True
Environment:
- OS: Linux (Pop_OS/Ubuntu)
- OS Version: 6.9.3/22.04
- Python: Python 3.10
Additional context
Correctly inheriting collections.abc.Mapping
would also improve Python static type analysis tooling (eg Ruff)