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

__delitem__ not as expected for enum key-type DictConfig #554

Closed
Jasha10 opened this issue Feb 22, 2021 · 0 comments · Fixed by #484
Closed

__delitem__ not as expected for enum key-type DictConfig #554

Jasha10 opened this issue Feb 22, 2021 · 0 comments · Fixed by #484
Labels
bug Something isn't working

Comments

@Jasha10
Copy link
Collaborator

Jasha10 commented Feb 22, 2021

Given a config with Enum key-type:

from enum import Enum
from omegaconf import DictConfig

class Enum1(Enum):
    FOO = 1
    BAR = 2

cfg = DictConfig({Enum1.FOO: 'enum key'}, key_type=Enum1)

normally it is possible to use either the enum value Enum1.FOO or the str value "FOO" for access:

>>> cfg[Enum1.FOO]
'enum key'
>>> cfg["FOO"]
'enum key'

But using string access fails when __delitem__ is called:

>>> del cfg["FOO"]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jasha10/omegaconf/omegaconf/dictconfig.py", line 405, in __delitem__
    del self.__dict__["_content"][key]
KeyError: 'FOO'

The __delitem__ works as expected when called with an enum value:

>>> del cfg[Enum1.FOO]
>>> assert Enum1.FOO not in cfg

Expected behavior:

The __delitem__ method should behave the same as __setitem__ and __getitem__: when a DictConfig instance has enum-typed keys, either a string or an enum member should be usable to specify what index will be deleted.

@Jasha10 Jasha10 added the bug Something isn't working label Feb 22, 2021
@omry omry closed this as completed in #484 Feb 22, 2021
odelalleau pushed a commit to odelalleau/omegaconf that referenced this issue Mar 26, 2021
comments

fixed docs

fixed notebook

enum_to_str: convert enum keys, not just enum values (omry#549)

README: Fix typo in "What's new" url

rebase against master

separate tests for struct mode and non-struct mode

news fragment for omry#554

one key per supported key type

Co-authored-by: Omry Yadan <[email protected]>

test None value for struct flag

special case parametrizing for struct_mode

change struct_mode test order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant