Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider inheriting from collections.Mapping and Sequence #114

Closed
omry opened this issue Jan 1, 2020 · 5 comments
Closed

Consider inheriting from collections.Mapping and Sequence #114

omry opened this issue Jan 1, 2020 · 5 comments

Comments

@omry
Copy link
Owner

omry commented Jan 1, 2020

No description provided.

@omry omry added this to the OmegaConf 2.0.0 milestone Jan 1, 2020
@omry
Copy link
Owner Author

omry commented Jan 4, 2020

Landed with typing.MutableMapping and typing.MutableSequence which are just as good.

@omry omry closed this as completed Jan 4, 2020
@poweic
Copy link

poweic commented Jun 21, 2020

Hi @omry,

I was trying to use isinstance and collections.abc.Mapping to check if an OmegaConf variable is a dictionary-like object but both isinstance(cfg, collections.abc.Mapping) and isinstance(cfg, collections.abc.MutableMapping) returned False.

Other packages like attrdict and dotmap inherit from collections.abc.Mapping.

Have you considered inheriting from collections.abc.Mapping? Thanks

In [1]: import omegaconf

In [2]: import collections

In [3]: isinstance(omegaconf.OmegaConf.create({"x": 10}), collections.abc.Mapping)
Out[3]: False

In [4]: isinstance(omegaconf.OmegaConf.create({"x": 10}), collections.abc.MutableMapping)
Out[4]: False

In [5]: from attrdict import AttrDict

In [6]: isinstance(AttrDict(x), collections.abc.Mapping)
Out[6]: True

In [7]: isinstance(AttrDict(x), collections.abc.MutableMapping)
Out[7]: True

In [8]: from dotmap import DotMap

In [9]: isinstance(DotMap({"x": 10}), collections.abc.Mapping)
Out[9]: True

In [10]: isinstance(DotMap({"x": 10}), collections.abc.MutableMapping)
Out[10]: True

In [11]: AttrDict?
Init signature: AttrDict(*args, **kwargs)
Docstring:      A dict that implements MutableAttr.
File:           ~/anaconda3/envs/xxx/lib/python3.7/site-packages/attrdict/dictionary.py
Type:           ABCMeta
Subclasses:

In [12]: DotMap?
Init signature: DotMap(*args, **kwargs)
Docstring:      Dictionary that remembers insertion order
File:           ~/anaconda3/envs/xxx/lib/python3.7/site-packages/dotmap/__init__.py
Type:           ABCMeta
Subclasses:

In [13]: omegaconf.dictconfig.DictConfig?
Init signature: omegaconf.dictconfig.DictConfig(content, parent=None)
Docstring:      <no docstring>
File:           ~/anaconda3/envs/xxx/lib/python3.7/site-packages/omegaconf/dictconfig.py
Type:           type
Subclasses:

@omry
Copy link
Owner Author

omry commented Jun 21, 2020

@poweic, did you test on 2.0?

@poweic
Copy link

poweic commented Jun 22, 2020

My bad 😅 Not sure why I got an older version of omegaconf (1.4.1) with freshly installed hydra-core. But they works in 2.0! Thanks!! 😃

@omry
Copy link
Owner Author

omry commented Jun 22, 2020

Hydra 0.11 is not compatible with OmegaConf 1.4.
Check the 1.0 pre release if you want to use Hydra and OmegaConf 2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants