From 4a765f249d59e58c6ca362b0b7e1d9f67e50c83d Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Tue, 9 Feb 2021 00:20:11 -0800 Subject: [PATCH 1/3] added list in test with interpolation --- tests/test_basic_ops_list.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_basic_ops_list.py b/tests/test_basic_ops_list.py index e8cd8c683..2bed50cc1 100644 --- a/tests/test_basic_ops_list.py +++ b/tests/test_basic_ops_list.py @@ -114,6 +114,11 @@ def test_in_list() -> None: assert "blah" not in c +def test_in_with_interpolation() -> None: + c = OmegaConf.create({"a": ["${b}"], "b": 10}) + assert 10 in c.a + + def test_list_config_with_list() -> None: c = OmegaConf.create([]) assert isinstance(c, ListConfig) From 479a01edb85669649c01a1497f6bdc25eb1e2ea7 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Tue, 9 Feb 2021 12:28:55 -0800 Subject: [PATCH 2/3] add benchmark to list __contains__ --- benchmark/benchmark.py | 49 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/benchmark/benchmark.py b/benchmark/benchmark.py index fbf8bc62a..67eaa0d3e 100644 --- a/benchmark/benchmark.py +++ b/benchmark/benchmark.py @@ -1,4 +1,4 @@ -from typing import Any, Dict +from typing import Any, Dict, List from pytest import lazy_fixture # type: ignore from pytest import fixture, mark, param @@ -6,7 +6,7 @@ from omegaconf import OmegaConf -def build( +def build_dict( d: Dict[str, Any], depth: int, width: int, leaf_value: Any = 1 ) -> Dict[str, Any]: if depth == 0: @@ -16,24 +16,28 @@ def build( for i in range(width): c: Dict[str, Any] = {} d[f"key_{i}"] = c - build(c, depth - 1, width, leaf_value) + build_dict(c, depth - 1, width, leaf_value) return d +def build_list(length: int, val: Any = 1) -> List[int]: + return [val] * length + + @fixture(scope="module") def large_dict() -> Any: - return build({}, 11, 2) + return build_dict({}, 11, 2) @fixture(scope="module") def small_dict() -> Any: - return build({}, 5, 2) + return build_dict({}, 5, 2) @fixture(scope="module") def dict_with_list_leaf() -> Any: - return build({}, 5, 2, leaf_value=[1, 2]) + return build_dict({}, 5, 2, leaf_value=[1, 2]) @fixture(scope="module") @@ -56,6 +60,16 @@ def merge_data(small_dict: Any) -> Any: return [OmegaConf.create(small_dict) for _ in range(5)] +@fixture(scope="module") +def small_list() -> Any: + return build_list(3, 1) + + +@fixture(scope="module") +def small_listconfig(small_list: Any) -> Any: + return OmegaConf.create(small_list) + + @mark.parametrize( "data", [ @@ -79,3 +93,26 @@ def test_omegaconf_create(data: Any, benchmark: Any) -> None: ) def test_omegaconf_merge(merge_function: Any, merge_data: Any, benchmark: Any) -> None: benchmark(merge_function, merge_data) + + +@mark.parametrize( + "lst", + [ + lazy_fixture("small_list"), + lazy_fixture("small_listconfig"), + ], +) +def test_list_in(lst: List[Any], benchmark: Any) -> None: + benchmark(lambda seq, val: val in seq, lst, 10) + + +@mark.parametrize( + "lst", + [ + lazy_fixture("small_list"), + lazy_fixture("small_listconfig"), + ], +) +def test_list_iter(lst: List[Any], benchmark: Any) -> None: + # Performance is really bad for list config iteration, not sure why. + benchmark(lambda seq: [x for x in seq], lst) From 65f2d1b52a7f01f2708c0126eb1ca266f058f195 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Tue, 9 Feb 2021 12:38:50 -0800 Subject: [PATCH 3/3] Optimized ListConfig.__contains__() Speedup from 1200x slower than standard list contains to only 68 times slower :) --- news/529.feature | 1 + omegaconf/listconfig.py | 13 ++++++++++++- tests/test_basic_ops_list.py | 26 ++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 news/529.feature diff --git a/news/529.feature b/news/529.feature new file mode 100644 index 000000000..e5b05dbb4 --- /dev/null +++ b/news/529.feature @@ -0,0 +1 @@ +ListConfig.__contains__ optimized, about 15x faster in a benchmark diff --git a/omegaconf/listconfig.py b/omegaconf/listconfig.py index 1b6d2cc49..5c1aaee90 100644 --- a/omegaconf/listconfig.py +++ b/omegaconf/listconfig.py @@ -539,7 +539,18 @@ def __iadd__(self, other: Iterable[Any]) -> "ListConfig": return self def __contains__(self, item: Any) -> bool: - for x in iter(self): + if self._is_none(): + raise TypeError( + "Cannot check if an item is in a ListConfig object representing None" + ) + if self._is_missing(): + raise MissingMandatoryValue( + "Cannot check if an item is in missing ListConfig" + ) + + lst = self.__dict__["_content"] + for x in lst: + x = x._dereference_node() if x == item: return True return False diff --git a/tests/test_basic_ops_list.py b/tests/test_basic_ops_list.py index 2bed50cc1..ef7bddb84 100644 --- a/tests/test_basic_ops_list.py +++ b/tests/test_basic_ops_list.py @@ -119,6 +119,32 @@ def test_in_with_interpolation() -> None: assert 10 in c.a +@pytest.mark.parametrize( + ("lst", "expected"), + [ + pytest.param( + ListConfig(content=None), + pytest.raises( + TypeError, + match="Cannot check if an item is in a ListConfig object representing None", + ), + id="ListConfig(None)", + ), + pytest.param( + ListConfig(content="???"), + pytest.raises( + MissingMandatoryValue, + match="Cannot check if an item is in missing ListConfig", + ), + id="ListConfig(???)", + ), + ], +) +def test_not_in_special_lists(lst: Any, expected: Any) -> None: + with expected: + "foo" not in lst + + def test_list_config_with_list() -> None: c = OmegaConf.create([]) assert isinstance(c, ListConfig)