From e6120586bce6410fcb7584d97300fd3589b2be9e Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 16:42:35 +0800
Subject: [PATCH 01/27] clean up types

---
 src/monty/design_patterns.py | 7 +++++--
 src/monty/fractions.py       | 5 ++++-
 src/monty/functools.py       | 7 ++++++-
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/monty/design_patterns.py b/src/monty/design_patterns.py
index a99a8a6b7..074505c9a 100644
--- a/src/monty/design_patterns.py
+++ b/src/monty/design_patterns.py
@@ -7,9 +7,12 @@
 import inspect
 import os
 from functools import wraps
-from typing import Any, Dict, Hashable, Tuple, TypeVar
+from typing import TYPE_CHECKING, TypeVar
 from weakref import WeakValueDictionary
 
+if TYPE_CHECKING:
+    from typing import Any
+
 
 def singleton(cls):
     """
@@ -95,7 +98,7 @@ def new_init(self: Any, *args: Any, **kwargs: Any) -> None:
             orig_init(self, *args, **kwargs)
             self._initialized = True
 
-    def reduce(self: Any) -> Tuple[type, Tuple, Dict[str, Any]]:
+    def reduce(self: Any) -> tuple[type, tuple, dict[str, Any]]:
         for key, value in cache.items():
             if value is self:
                 cls, args = key
diff --git a/src/monty/fractions.py b/src/monty/fractions.py
index d29b79600..f0650e5a5 100644
--- a/src/monty/fractions.py
+++ b/src/monty/fractions.py
@@ -5,7 +5,10 @@
 from __future__ import annotations
 
 import math
-from typing import Sequence
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from typing import Sequence
 
 
 def gcd(*numbers: int) -> int:
diff --git a/src/monty/functools.py b/src/monty/functools.py
index 32e55517c..3657e024b 100644
--- a/src/monty/functools.py
+++ b/src/monty/functools.py
@@ -11,8 +11,13 @@
 import tempfile
 from collections import namedtuple
 from functools import partial, wraps
-from typing import Any, Callable, Union
+from typing import TYPE_CHECKING
 
+if TYPE_CHECKING:
+    from typing import Any, Callable, Union
+
+
+# _CacheInfo doesn't seem to be used
 _CacheInfo = namedtuple("_CacheInfo", ["hits", "misses", "maxsize", "currsize"])
 
 

From 03fc2533c89ef42438f19706983d084051f3328d Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 16:43:06 +0800
Subject: [PATCH 02/27] remove unused _CacheInfo

---
 src/monty/functools.py | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/monty/functools.py b/src/monty/functools.py
index 3657e024b..969f0d506 100644
--- a/src/monty/functools.py
+++ b/src/monty/functools.py
@@ -9,7 +9,6 @@
 import signal
 import sys
 import tempfile
-from collections import namedtuple
 from functools import partial, wraps
 from typing import TYPE_CHECKING
 
@@ -17,10 +16,6 @@
     from typing import Any, Callable, Union
 
 
-# _CacheInfo doesn't seem to be used
-_CacheInfo = namedtuple("_CacheInfo", ["hits", "misses", "maxsize", "currsize"])
-
-
 class _HashedSeq(list):  # pylint: disable=C0205
     """
     This class guarantees that hash() will be called no more than once

From 9b1192256fb7a93c6ae21da53d247210e7678e04 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 17:40:20 +0800
Subject: [PATCH 03/27] clean up comments and tests

---
 src/monty/collections.py  | 35 +++++++-------
 tests/test_collections.py | 97 +++++++++++++++++++++------------------
 2 files changed, 70 insertions(+), 62 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 812bff832..2f4f68c3f 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -20,7 +20,7 @@ def tree() -> collections.defaultdict:
 
     Usage:
         x = tree()
-        x['a']['b']['c'] = 1
+        x["a"]["b"]["c"] = 1
 
     Returns:
         A tree.
@@ -31,7 +31,7 @@ def tree() -> collections.defaultdict:
 class frozendict(dict):
     """
     A dictionary that does not permit changes. The naming
-    violates PEP8 to be consistent with standard Python's "frozenset" naming.
+    violates PEP 8 to be consistent with standard Python's "frozenset" naming.
     """
 
     def __init__(self, *args, **kwargs) -> None:
@@ -55,7 +55,7 @@ def update(self, *args, **kwargs) -> None:
 
 
 class Namespace(dict):
-    """A dictionary that does not permit to redefine its keys."""
+    """A dictionary that does not permit changing its values."""
 
     def __init__(self, *args, **kwargs) -> None:
         """
@@ -67,7 +67,7 @@ def __init__(self, *args, **kwargs) -> None:
 
     def __setitem__(self, key: Any, val: Any) -> None:
         if key in self:
-            raise KeyError(f"Cannot overwrite existent key: {key!s}")
+            raise KeyError(f"Cannot overwrite existing key: {key!s}")
 
         dict.__setitem__(self, key, val)
 
@@ -83,14 +83,14 @@ def update(self, *args, **kwargs) -> None:
 
 class AttrDict(dict):
     """
-    Allows to access dict keys as obj.foo in addition
-    to the traditional way obj['foo']"
+    Allows to access values as dct.key in addition
+    to the traditional way dct["key"]
 
     Examples:
-        >>> d = AttrDict(foo=1, bar=2)
-        >>> assert d["foo"] == d.foo
-        >>> d.bar = "hello"
-        >>> assert d.bar == "hello"
+        >>> dct = AttrDict(foo=1, bar=2)
+        >>> assert dct["foo"] == dct.foo
+        >>> dct.bar = "hello"
+        >>> assert dct.bar == "hello"
     """
 
     def __init__(self, *args, **kwargs) -> None:
@@ -114,9 +114,9 @@ def copy(self) -> Self:
 class FrozenAttrDict(frozendict):
     """
     A dictionary that:
-        * does not permit changes.
-        * Allows to access dict keys as obj.foo in addition
-          to the traditional way obj['foo']
+        - Does not permit changes.
+        - Allows to access values as dct.key in addition
+          to the traditional way dct["key"]
     """
 
     def __init__(self, *args, **kwargs) -> None:
@@ -186,7 +186,6 @@ def __getattribute__(self, name: str) -> Any:
         try:
             return super().__getattribute__(name)
         except AttributeError:
-            # raise
             try:
                 a = self._mongo_dict_[name]
                 if isinstance(a, collections.abc.Mapping):
@@ -232,8 +231,8 @@ def dict2namedtuple(*args, **kwargs) -> tuple:
         - Don't use this function in code in which memory and performance are
           crucial since a dict is needed to instantiate the tuple!
     """
-    d = collections.OrderedDict(*args)
-    d.update(**kwargs)
+    dct = collections.OrderedDict(*args)
+    dct.update(**kwargs)
     return collections.namedtuple(
-        typename="dict2namedtuple", field_names=list(d.keys())
-    )(**d)
+        typename="dict2namedtuple", field_names=list(dct.keys())
+    )(**dct)
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 58d2af9e7..a8e65b41f 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -9,47 +9,56 @@
 TEST_DIR = os.path.join(os.path.dirname(__file__), "test_files")
 
 
-class TestFrozenDict:
-    def test_frozen_dict(self):
-        d = frozendict({"hello": "world"})
-        with pytest.raises(KeyError):
-            d["k"] == "v"
-        assert d["hello"] == "world"
-
-    def test_namespace_dict(self):
-        d = Namespace(foo="bar")
-        d["hello"] = "world"
-        assert d["foo"] == "bar"
-        with pytest.raises(KeyError):
-            d.update({"foo": "spam"})
-
-    def test_attr_dict(self):
-        d = AttrDict(foo=1, bar=2)
-        assert d.bar == 2
-        assert d["foo"] == d.foo
-        d.bar = "hello"
-        assert d["bar"] == "hello"
-
-    def test_frozen_attrdict(self):
-        d = FrozenAttrDict({"hello": "world", 1: 2})
-        assert d["hello"] == "world"
-        assert d.hello == "world"
-        with pytest.raises(KeyError):
-            d["updating"] == 2
-
-        with pytest.raises(KeyError):
-            d["foo"] = "bar"
-        with pytest.raises(KeyError):
-            d.foo = "bar"
-        with pytest.raises(KeyError):
-            d.hello = "new"
-
-
-class TestTree:
-    def test_tree(self):
-        x = tree()
-        x["a"]["b"]["c"]["d"] = 1
-        assert "b" in x["a"]
-        assert "c" not in x["a"]
-        assert "c" in x["a"]["b"]
-        assert x["a"]["b"]["c"]["d"] == 1
+def test_tree():
+    x = tree()
+    x["a"]["b"]["c"]["d"] = 1
+    assert "b" in x["a"]
+    assert "c" not in x["a"]
+    assert "c" in x["a"]["b"]
+    assert x["a"]["b"]["c"]["d"] == 1
+
+
+def test_frozendict():
+    dct = frozendict({"hello": "world"})
+    assert dct["hello"] == "world"
+
+    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+        dct["key"] == "val"
+
+    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+        dct.update(key="value")
+
+
+def test_namespace_dict():
+    dct = Namespace(foo="bar")
+    dct["hello"] = "world"
+    assert dct["key"] == "val"
+
+    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+        dct["key"] = "val"
+    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+        dct.update({"key": "val"})
+
+
+def test_attr_dict():
+    dct = AttrDict(foo=1, bar=2)
+    assert dct.bar == 2
+    assert dct["foo"] is dct.foo
+    dct.bar = "hello"
+    assert dct["bar"] == "hello"
+
+
+def test_frozen_attrdict():
+    dct = FrozenAttrDict({"hello": "world", 1: 2})
+    assert dct["hello"] == "world"
+    assert dct.hello == "world"
+
+    # Test adding item
+    with pytest.raises(KeyError, match="You cannot modify attribute"):
+        dct["foo"] = "bar"
+    with pytest.raises(KeyError, match="You cannot modify attribute"):
+        dct.foo = "bar"
+
+    # Test modifying existing item
+    with pytest.raises(KeyError, match="You cannot modify attribute"):
+        dct.hello = "new"

From ecf19a5a55ee7cd6cff257a6996db7f7e5a6ecba Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 19:36:15 +0800
Subject: [PATCH 04/27] add test for dict2namedtuple

---
 src/monty/collections.py  |  8 ++++----
 tests/test_collections.py | 34 ++++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 2f4f68c3f..7a3085c41 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -216,11 +216,11 @@ def dict2namedtuple(*args, **kwargs) -> tuple:
     Helper function to create a class `namedtuple` from a dictionary.
 
     Examples:
-        >>> t = dict2namedtuple(foo=1, bar="hello")
-        >>> assert t.foo == 1 and t.bar == "hello"
+        >>> tpl = dict2namedtuple(foo=1, bar="hello")
+        >>> assert tpl.foo == 1 and tpl.bar == "hello"
 
-        >>> t = dict2namedtuple([("foo", 1), ("bar", "hello")])
-        >>> assert t[0] == t.foo and t[1] == t.bar
+        >>> tpl = dict2namedtuple([("foo", 1), ("bar", "hello")])
+        >>> assert tpl[0] is tpl.foo and t[1] is tpl.bar
 
     Warnings:
         - The order of the items in the namedtuple is not deterministic if
diff --git a/tests/test_collections.py b/tests/test_collections.py
index a8e65b41f..37c96673a 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -1,10 +1,18 @@
 from __future__ import annotations
 
 import os
+from collections import namedtuple
 
 import pytest
 
-from monty.collections import AttrDict, FrozenAttrDict, Namespace, frozendict, tree
+from monty.collections import (
+    AttrDict,
+    FrozenAttrDict,
+    Namespace,
+    dict2namedtuple,
+    frozendict,
+    tree,
+)
 
 TEST_DIR = os.path.join(os.path.dirname(__file__), "test_files")
 
@@ -30,7 +38,7 @@ def test_frozendict():
 
 
 def test_namespace_dict():
-    dct = Namespace(foo="bar")
+    dct = Namespace(key="val")
     dct["hello"] = "world"
     assert dct["key"] == "val"
 
@@ -38,6 +46,8 @@ def test_namespace_dict():
         dct["key"] = "val"
     with pytest.raises(KeyError, match="Cannot overwrite existing key"):
         dct.update({"key": "val"})
+    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+        dct |= {"key": "val"}
 
 
 def test_attr_dict():
@@ -62,3 +72,23 @@ def test_frozen_attrdict():
     # Test modifying existing item
     with pytest.raises(KeyError, match="You cannot modify attribute"):
         dct.hello = "new"
+    with pytest.raises(KeyError, match="You cannot modify attribute"):
+        dct["hello"] = "new"
+
+
+def test_mongo_dict():
+    """TODO: add test"""
+
+
+def test_dict2namedtuple():
+    # Init from dict
+    tpl = dict2namedtuple(foo=1, bar="hello")
+    assert isinstance(tpl, tuple)
+    assert tpl.foo == 1 and tpl.bar == "hello"
+
+    # Init from list of tuples
+    tpl = dict2namedtuple([("foo", 1), ("bar", "hello")])
+    assert isinstance(tpl, tuple)
+    assert tpl[0] == 1
+    assert tpl[1] == "hello"
+    assert tpl[0] is tpl.foo and tpl[1] is tpl.bar

From bad69382244449924f315da75b41349d9df9aeee Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 19:49:38 +0800
Subject: [PATCH 05/27] add test_mongo_dict

---
 src/monty/collections.py  | 10 +++++-----
 tests/test_collections.py | 11 +++++++++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 7a3085c41..a972a27cc 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -151,11 +151,11 @@ class MongoDict:
     a nested dict interactively (e.g. documents extracted from a MongoDB
     database).
 
-    >>> m = MongoDict({'a': {'b': 1}, 'x': 2})
-    >>> assert m.a.b == 1 and m.x == 2
-    >>> assert "a" in m and "b" in m.a
-    >>> m["a"]
-    {'b': 1}
+    >>> m_dct = MongoDict({"a": {"b": 1}, "x": 2})
+    >>> assert m_dct.a.b == 1 and m_dct.x == 2
+    >>> assert "a" in m_dct and "b" in m_dct.a
+    >>> m_dct["a"]
+    {"b": 1}
 
     Notes:
         Cannot inherit from ABC collections.Mapping because otherwise
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 37c96673a..f0a59828a 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -1,13 +1,13 @@
 from __future__ import annotations
 
 import os
-from collections import namedtuple
 
 import pytest
 
 from monty.collections import (
     AttrDict,
     FrozenAttrDict,
+    MongoDict,
     Namespace,
     dict2namedtuple,
     frozendict,
@@ -54,6 +54,8 @@ def test_attr_dict():
     dct = AttrDict(foo=1, bar=2)
     assert dct.bar == 2
     assert dct["foo"] is dct.foo
+
+    # Test accessing values
     dct.bar = "hello"
     assert dct["bar"] == "hello"
 
@@ -77,7 +79,12 @@ def test_frozen_attrdict():
 
 
 def test_mongo_dict():
-    """TODO: add test"""
+    m_dct = MongoDict({"a": {"b": 1}, "x": 2})
+    assert m_dct.a.b == 1
+    assert m_dct.x == 2
+    assert "a" in m_dct
+    assert "b" in m_dct.a
+    assert m_dct["a"] == {"b": 1}
 
 
 def test_dict2namedtuple():

From dbcc676121465ec2f16b1f85e92e290e23c84ad8 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 19:56:31 +0800
Subject: [PATCH 06/27] some minor comment clean up

---
 src/monty/collections.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index a972a27cc..992be719e 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -31,7 +31,7 @@ def tree() -> collections.defaultdict:
 class frozendict(dict):
     """
     A dictionary that does not permit changes. The naming
-    violates PEP 8 to be consistent with standard Python's "frozenset" naming.
+    violates PEP 8 to be consistent with the built-in "frozenset" naming.
     """
 
     def __init__(self, *args, **kwargs) -> None:
@@ -88,7 +88,7 @@ class AttrDict(dict):
 
     Examples:
         >>> dct = AttrDict(foo=1, bar=2)
-        >>> assert dct["foo"] == dct.foo
+        >>> assert dct["foo"] is dct.foo
         >>> dct.bar = "hello"
         >>> assert dct.bar == "hello"
     """

From 5f4bbea764d8f4f84dc6f6ec06c9cc9f837d01d7 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 20:00:20 +0800
Subject: [PATCH 07/27] remove unused test dir

---
 tests/test_collections.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tests/test_collections.py b/tests/test_collections.py
index f0a59828a..42cc8ea7d 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -1,7 +1,5 @@
 from __future__ import annotations
 
-import os
-
 import pytest
 
 from monty.collections import (
@@ -14,8 +12,6 @@
     tree,
 )
 
-TEST_DIR = os.path.join(os.path.dirname(__file__), "test_files")
-
 
 def test_tree():
     x = tree()

From ecae809d0a42f75868afe3c420f3dc040d7cf8ad Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 20:08:25 +0800
Subject: [PATCH 08/27] keyerror to type error

---
 src/monty/collections.py  |  8 ++++----
 tests/test_collections.py | 35 ++++++++++++++++++++++-------------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 992be719e..3b0425c07 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -43,7 +43,7 @@ def __init__(self, *args, **kwargs) -> None:
         dict.__init__(self, *args, **kwargs)
 
     def __setitem__(self, key: Any, val: Any) -> None:
-        raise KeyError(f"Cannot overwrite existing key: {str(key)}")
+        raise TypeError(f"Cannot overwrite existing key: {str(key)}")
 
     def update(self, *args, **kwargs) -> None:
         """
@@ -51,7 +51,7 @@ def update(self, *args, **kwargs) -> None:
             args: Passthrough arguments for standard dict.
             kwargs: Passthrough keyword arguments for standard dict.
         """
-        raise KeyError(f"Cannot update a {self.__class__.__name__}")
+        raise TypeError(f"Cannot update a {self.__class__.__name__}")
 
 
 class Namespace(dict):
@@ -67,7 +67,7 @@ def __init__(self, *args, **kwargs) -> None:
 
     def __setitem__(self, key: Any, val: Any) -> None:
         if key in self:
-            raise KeyError(f"Cannot overwrite existing key: {key!s}")
+            raise TypeError(f"Cannot overwrite existing key: {key!s}")
 
         dict.__setitem__(self, key, val)
 
@@ -137,7 +137,7 @@ def __getattribute__(self, name: str) -> Any:
                 raise AttributeError(str(exc))
 
     def __setattr__(self, name: str, value: Any) -> None:
-        raise KeyError(
+        raise TypeError(
             f"You cannot modify attribute {name} of {self.__class__.__name__}"
         )
 
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 42cc8ea7d..a6540ec20 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -22,27 +22,34 @@ def test_tree():
     assert x["a"]["b"]["c"]["d"] == 1
 
 
-def test_frozendict():
+def test_frozendict():  # DEBUG
     dct = frozendict({"hello": "world"})
+    assert isinstance(dct, dict)
     assert dct["hello"] == "world"
 
-    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
         dct["key"] == "val"
 
-    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
-        dct.update(key="value")
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+        dct.update(key="val")
+
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+        dct |= {"key": "val"}
 
 
-def test_namespace_dict():
+def test_namespace_dict():  # DEBUG
     dct = Namespace(key="val")
+    assert isinstance(dct, dict)
     dct["hello"] = "world"
     assert dct["key"] == "val"
 
-    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
         dct["key"] = "val"
-    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
         dct.update({"key": "val"})
-    with pytest.raises(KeyError, match="Cannot overwrite existing key"):
+
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
         dct |= {"key": "val"}
 
 
@@ -56,21 +63,23 @@ def test_attr_dict():
     assert dct["bar"] == "hello"
 
 
-def test_frozen_attrdict():
+def test_frozen_attrdict():  # DEBUG
     dct = FrozenAttrDict({"hello": "world", 1: 2})
+    assert isinstance(dct, dict)
     assert dct["hello"] == "world"
     assert dct.hello == "world"
+    assert dct["hello"] is dct.hello
 
     # Test adding item
-    with pytest.raises(KeyError, match="You cannot modify attribute"):
+    with pytest.raises(TypeError, match="You cannot modify attribute"):
         dct["foo"] = "bar"
-    with pytest.raises(KeyError, match="You cannot modify attribute"):
+    with pytest.raises(TypeError, match="You cannot modify attribute"):
         dct.foo = "bar"
 
     # Test modifying existing item
-    with pytest.raises(KeyError, match="You cannot modify attribute"):
+    with pytest.raises(TypeError, match="You cannot modify attribute"):
         dct.hello = "new"
-    with pytest.raises(KeyError, match="You cannot modify attribute"):
+    with pytest.raises(TypeError, match="You cannot modify attribute"):
         dct["hello"] = "new"
 
 

From 284ce88e227cee6affa61314fcf2ed498a864d78 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 13 Nov 2024 22:45:53 +0800
Subject: [PATCH 09/27] add place holder

---
 src/monty/collections.py  |  68 ++++++++++++++++++++++--
 tests/test_collections.py | 106 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 167 insertions(+), 7 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 3b0425c07..37bb02bfc 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -1,14 +1,24 @@
 """
-Useful collection classes, e.g., tree, frozendict, etc.
+Useful collection classes:
+    - tree: A recursive `defaultdict` for creating nested dictionaries
+        with default values.
+    - frozendict: An immutable dictionary.
+    - Namespace: A dict doesn't allow changing values, but could
+        add new keys,
+    - AttrDict: A dict whose values could be access as `dct.key`.
+    - FrozenAttrDict: An immutable version of `AttrDict`.
+    - MongoDict: A dict-like object whose values are nested dicts
+        could be accessed as attributes.
 """
 
 from __future__ import annotations
 
 import collections
+from abc import ABC
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
-    from typing import Any, Iterable
+    from typing import Any, ClassVar, Iterable
 
     from typing_extensions import Self
 
@@ -28,6 +38,56 @@ def tree() -> collections.defaultdict:
     return collections.defaultdict(tree)
 
 
+class ControlledDict(collections.UserDict, ABC):
+    """
+    A base dictionary class with configurable mutability.
+
+    Attributes:
+        allow_add (bool): Whether new keys can be added.
+        allow_del (bool): Whether existing keys can be deleted.
+        allow_update (bool): Whether existing keys can be updated.
+    """
+
+    allow_add: ClassVar[bool] = True
+    allow_del: ClassVar[bool] = True
+    allow_update: ClassVar[bool] = True
+
+    def __setitem__(self, key, value):
+        """Forbid set when self.ADD is False."""
+        if key not in self.data and not self.allow_add:
+            raise TypeError("Cannot add new key, because add is disabled.")
+
+        super().__setitem__(key, value)
+
+    # def __ior__(self, other):
+    #     # Handle the |= operator for dictionary merging
+    #     for key in other:
+    #         if key not in self.data and not self.ADD:
+    #             raise TypeError(
+    #                 f"Cannot add new key using |= operator: {key!r} because ADD is set to False"
+    #             )
+
+    #     return super().__ior__(other)
+
+    def update(self, *args, **kwargs):
+        # For each key in the provided dictionary, check if it's a new key
+        for key in dict(*args, **kwargs):
+            if key not in self.data and not self.allow_add:
+                raise KeyError(
+                    f"Cannot add new key using update: {key!r} because ADD is set to False"
+                )
+
+        super().update(*args, **kwargs)
+
+    def setdefault(self, key, default=None):
+        if key not in self.data and not self.allow_add:
+            raise TypeError(
+                f"Cannot add new key using setdefault: {key!r} because ADD is set to False"
+            )
+
+        return super().setdefault(key, default)
+
+
 class frozendict(dict):
     """
     A dictionary that does not permit changes. The naming
@@ -43,7 +103,7 @@ def __init__(self, *args, **kwargs) -> None:
         dict.__init__(self, *args, **kwargs)
 
     def __setitem__(self, key: Any, val: Any) -> None:
-        raise TypeError(f"Cannot overwrite existing key: {str(key)}")
+        raise TypeError(f"{type(self).__name__} does not support item assignment")
 
     def update(self, *args, **kwargs) -> None:
         """
@@ -54,7 +114,7 @@ def update(self, *args, **kwargs) -> None:
         raise TypeError(f"Cannot update a {self.__class__.__name__}")
 
 
-class Namespace(dict):
+class Namespace(dict):  # TODO: this name is a bit confusing, deprecate it?
     """A dictionary that does not permit changing its values."""
 
     def __init__(self, *args, **kwargs) -> None:
diff --git a/tests/test_collections.py b/tests/test_collections.py
index a6540ec20..fe672d9e1 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -4,6 +4,7 @@
 
 from monty.collections import (
     AttrDict,
+    ControlledDict,
     FrozenAttrDict,
     MongoDict,
     Namespace,
@@ -22,22 +23,121 @@ def test_tree():
     assert x["a"]["b"]["c"]["d"] == 1
 
 
-def test_frozendict():  # DEBUG
+class TestControlledDict:
+    def test_add_allowed(self):
+        dct = ControlledDict(a=1)
+        dct.allow_add = True
+
+        dct["b"] = 2
+        assert dct["b"] == 2
+
+        dct |= {"c": 3}
+        assert dct["c"] == 3
+
+        dct.update(d=4)
+        assert dct["d"] == 4
+
+        dct.setdefault("e", 5)
+        assert dct["e"] == 5
+
+    def test_add_disabled(self):
+        dct = ControlledDict(a=1)
+        dct.allow_add = False
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct["b"] = 2
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct.update(a=2)
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct.setdefault("a", 2)
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct |= {"b": 2}
+
+    def test_del_allowed(self):
+        dct = ControlledDict(a=1, b=2, c=3, d=4)
+        dct.allow_del = True
+
+        del dct["a"]
+        assert "a" not in dct
+
+        val = dct.pop("b")
+        assert val == 2 and "b" not in dct
+
+        val = dct.popitem()
+        assert val == ("c", 3) and "c" not in dct
+
+        dct.clear()
+        assert dct == {}
+
+    def test_del_disabled(self):
+        dct = ControlledDict(a=1)
+        dct.allow_del = False
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            del dct["a"]
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct.pop("a")
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct.popitem()
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct.clear()
+
+    def test_update_allowed(self):
+        dct = ControlledDict(a=1)
+        dct.allow_update = True
+
+        dct["a"] = 2
+        assert dct["a"] == 2
+
+        dct |= {"a": 3}
+        assert dct["a"] == 3
+
+    def test_update_disabled(self):
+        dct = ControlledDict(a=1)
+        dct.allow_update = False
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct["a"] = 2
+
+        with pytest.raises(TypeError):  # TODO: add msg
+            dct |= {"a": 3}
+
+
+def test_frozendict():
     dct = frozendict({"hello": "world"})
     assert isinstance(dct, dict)
     assert dct["hello"] == "world"
 
+    # Test setter
     with pytest.raises(TypeError, match="Cannot overwrite existing key"):
         dct["key"] == "val"
 
+    # Test update
     with pytest.raises(TypeError, match="Cannot overwrite existing key"):
         dct.update(key="val")
 
+    # Test inplace-or (|=)
     with pytest.raises(TypeError, match="Cannot overwrite existing key"):
         dct |= {"key": "val"}
 
+    # TODO: from this point we need a different error message
+
+    # Test pop
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+        dct.pop("key")
+
+    # Test delete
+    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+        del dct["key"]
+
 
-def test_namespace_dict():  # DEBUG
+def test_namespace_dict():
     dct = Namespace(key="val")
     assert isinstance(dct, dict)
     dct["hello"] = "world"
@@ -63,7 +163,7 @@ def test_attr_dict():
     assert dct["bar"] == "hello"
 
 
-def test_frozen_attrdict():  # DEBUG
+def test_frozen_attrdict():
     dct = FrozenAttrDict({"hello": "world", 1: 2})
     assert isinstance(dct, dict)
     assert dct["hello"] == "world"

From affcfe99c3e73f2d82abde910052e25976364b36 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 12:10:11 +0800
Subject: [PATCH 10/27] docstring clean up

---
 src/monty/collections.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 37bb02bfc..5109734a9 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -2,6 +2,7 @@
 Useful collection classes:
     - tree: A recursive `defaultdict` for creating nested dictionaries
         with default values.
+    - ControlledDict: A base dict class with configurable mutability.
     - frozendict: An immutable dictionary.
     - Namespace: A dict doesn't allow changing values, but could
         add new keys,
@@ -53,7 +54,7 @@ class ControlledDict(collections.UserDict, ABC):
     allow_update: ClassVar[bool] = True
 
     def __setitem__(self, key, value):
-        """Forbid set when self.ADD is False."""
+        """Forbid set when self.allow_add is False."""
         if key not in self.data and not self.allow_add:
             raise TypeError("Cannot add new key, because add is disabled.")
 
@@ -273,7 +274,7 @@ def __dir__(self) -> list:
 
 def dict2namedtuple(*args, **kwargs) -> tuple:
     """
-    Helper function to create a class `namedtuple` from a dictionary.
+    Helper function to create a `namedtuple` from a dictionary.
 
     Examples:
         >>> tpl = dict2namedtuple(foo=1, bar="hello")
@@ -284,12 +285,11 @@ def dict2namedtuple(*args, **kwargs) -> tuple:
 
     Warnings:
         - The order of the items in the namedtuple is not deterministic if
-          kwargs are used.
-          namedtuples, however, should always be accessed by attribute hence
-          this behaviour should not represent a serious problem.
+          `kwargs` are used. namedtuples, however, should always be accessed
+          by attribute hence this behaviour should not be a serious problem.
 
-        - Don't use this function in code in which memory and performance are
-          crucial since a dict is needed to instantiate the tuple!
+        - Don't use this function in code where memory and performance are
+          crucial, since a dict is needed to instantiate the tuple!
     """
     dct = collections.OrderedDict(*args)
     dct.update(**kwargs)

From 0f979b00ce8364fef8792a91dce7ed2971a886fb Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 12:48:23 +0800
Subject: [PATCH 11/27] clean up implementation and tests

---
 src/monty/collections.py  | 98 +++++++++++++++++++++++++++++++++------
 tests/test_collections.py | 74 ++++++++++++++++-------------
 2 files changed, 126 insertions(+), 46 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 5109734a9..d4ef8dbea 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -47,47 +47,115 @@ class ControlledDict(collections.UserDict, ABC):
         allow_add (bool): Whether new keys can be added.
         allow_del (bool): Whether existing keys can be deleted.
         allow_update (bool): Whether existing keys can be updated.
+
+    Configurable Operations:
+        This class allows controlling the following dictionary operations (refer to
+        https://docs.python.org/3.13/library/stdtypes.html#mapping-types-dict for details):
+
+        - Adding or updating items:
+            - setter method: `__setitem__`
+            - `setdefault`
+            - `update`
+            - Merging dictionaries using `|=`
+
+        - Deleting items:
+            - `del dict[key]`
+            - `pop(key)`
+            - `popitem`
+            - `clear()`
     """
 
     allow_add: ClassVar[bool] = True
     allow_del: ClassVar[bool] = True
     allow_update: ClassVar[bool] = True
 
+    # TODO: extract checkers
+
+    # Overriding add/update operations
     def __setitem__(self, key, value):
-        """Forbid set when self.allow_add is False."""
+        """Forbid adding or updating keys based on allow_add and allow_update."""
         if key not in self.data and not self.allow_add:
-            raise TypeError("Cannot add new key, because add is disabled.")
+            raise TypeError(
+                f"Cannot add new key {key!r}, because allow_add is set to False."
+            )
+        elif key in self.data and not self.allow_update:
+            raise TypeError(
+                f"Cannot update key {key!r}, because allow_update is set to False."
+            )
 
         super().__setitem__(key, value)
 
-    # def __ior__(self, other):
-    #     # Handle the |= operator for dictionary merging
-    #     for key in other:
-    #         if key not in self.data and not self.ADD:
-    #             raise TypeError(
-    #                 f"Cannot add new key using |= operator: {key!r} because ADD is set to False"
-    #             )
+    def __ior__(self, other):
+        """Handle the |= operator for dictionary updates."""
+        for key in other:
+            if key not in self.data and not self.allow_add:
+                raise TypeError(
+                    f"Cannot add new key using |= operator: {key!r}, because allow_add is set to False."
+                )
+            elif key in self.data and not self.allow_update:
+                raise TypeError(
+                    f"Cannot update key using |= operator: {key!r}, because allow_update is set to False."
+                )
 
-    #     return super().__ior__(other)
+        return super().__ior__(other)
 
     def update(self, *args, **kwargs):
-        # For each key in the provided dictionary, check if it's a new key
+        """Forbid adding or updating keys based on allow_add and allow_update."""
         for key in dict(*args, **kwargs):
             if key not in self.data and not self.allow_add:
-                raise KeyError(
-                    f"Cannot add new key using update: {key!r} because ADD is set to False"
+                raise TypeError(
+                    f"Cannot add new key {key!r} using update, because allow_add is set to False."
+                )
+            elif key in self.data and not self.allow_update:
+                raise TypeError(
+                    f"Cannot update key {key!r} using update, because allow_update is set to False."
                 )
 
         super().update(*args, **kwargs)
 
     def setdefault(self, key, default=None):
-        if key not in self.data and not self.allow_add:
+        """Forbid adding or updating keys based on allow_add and allow_update.
+
+        Note: if not allow_update, this method would NOT check whether the
+            new default value is the same as current value for efficiency.
+        """
+        if key not in self.data:
+            if not self.allow_add:
+                raise TypeError(
+                    f"Cannot add new key using setdefault: {key!r}, because allow_add is set to False."
+                )
+        elif not self.allow_update:
             raise TypeError(
-                f"Cannot add new key using setdefault: {key!r} because ADD is set to False"
+                f"Cannot update key using setdefault: {key!r}, because allow_update is set to False."
             )
 
         return super().setdefault(key, default)
 
+    # Overriding delete operations
+    def __delitem__(self, key):
+        """Forbid deleting keys when self.allow_del is False."""
+        if not self.allow_del:
+            raise TypeError(f"Cannot delete key {key!r}, because delete is disabled.")
+        super().__delitem__(key)
+
+    def pop(self, key, *args):
+        """Forbid popping keys when self.allow_del is False."""
+        if not self.allow_del:
+            raise TypeError(f"Cannot pop key {key!r}, because delete is disabled.")
+        return super().pop(key, *args)
+
+    def popitem(self):
+        """Forbid popping the last item when self.allow_del is False."""
+        if not self.allow_del:
+            raise TypeError("Cannot pop item, because delete is disabled.")
+        return super().popitem()
+
+    def clear(self):
+        """Forbid clearing the dictionary when self.allow_del is False."""
+        if not self.allow_del:
+            raise TypeError("Cannot clear dictionary, because delete is disabled.")
+        super().clear()
+
 
 class frozendict(dict):
     """
diff --git a/tests/test_collections.py b/tests/test_collections.py
index fe672d9e1..a071dee1a 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -44,17 +44,49 @@ def test_add_disabled(self):
         dct = ControlledDict(a=1)
         dct.allow_add = False
 
-        with pytest.raises(TypeError):  # TODO: add msg
+        with pytest.raises(TypeError, match="allow_add is set to False"):
             dct["b"] = 2
 
-        with pytest.raises(TypeError):  # TODO: add msg
-            dct.update(a=2)
+        with pytest.raises(TypeError, match="allow_add is set to False"):
+            dct.update(b=2)
 
-        with pytest.raises(TypeError):  # TODO: add msg
-            dct.setdefault("a", 2)
+        with pytest.raises(TypeError, match="allow_add is set to False"):
+            dct.setdefault("c", 2)
 
-        with pytest.raises(TypeError):  # TODO: add msg
-            dct |= {"b": 2}
+        with pytest.raises(TypeError, match="allow_add is set to False"):
+            dct |= {"d": 2}
+
+    def test_update_allowed(self):
+        dct = ControlledDict(a=1)
+        dct.allow_update = True
+
+        dct["a"] = 2
+        assert dct["a"] == 2
+
+        dct |= {"a": 3}
+        assert dct["a"] == 3
+
+        dct.update({"a": 4})
+        assert dct["a"] == 4
+
+        dct.setdefault("a", 5)  # existing key
+        assert dct["a"] == 4
+
+    def test_update_disabled(self):
+        dct = ControlledDict(a=1)
+        dct.allow_update = False
+
+        with pytest.raises(TypeError, match="allow_update is set to False"):
+            dct["a"] = 2
+
+        with pytest.raises(TypeError, match="allow_update is set to False"):
+            dct.update({"a": 3})
+
+        with pytest.raises(TypeError, match="allow_update is set to False"):
+            dct |= {"a": 4}
+
+        with pytest.raises(TypeError, match="allow_update is set to False"):
+            dct.setdefault("a", 5)
 
     def test_del_allowed(self):
         dct = ControlledDict(a=1, b=2, c=3, d=4)
@@ -76,38 +108,18 @@ def test_del_disabled(self):
         dct = ControlledDict(a=1)
         dct.allow_del = False
 
-        with pytest.raises(TypeError):  # TODO: add msg
+        with pytest.raises(TypeError, match="delete is disabled"):
             del dct["a"]
 
-        with pytest.raises(TypeError):  # TODO: add msg
+        with pytest.raises(TypeError, match="delete is disabled"):
             dct.pop("a")
 
-        with pytest.raises(TypeError):  # TODO: add msg
+        with pytest.raises(TypeError, match="delete is disabled"):
             dct.popitem()
 
-        with pytest.raises(TypeError):  # TODO: add msg
+        with pytest.raises(TypeError, match="delete is disabled"):
             dct.clear()
 
-    def test_update_allowed(self):
-        dct = ControlledDict(a=1)
-        dct.allow_update = True
-
-        dct["a"] = 2
-        assert dct["a"] == 2
-
-        dct |= {"a": 3}
-        assert dct["a"] == 3
-
-    def test_update_disabled(self):
-        dct = ControlledDict(a=1)
-        dct.allow_update = False
-
-        with pytest.raises(TypeError):  # TODO: add msg
-            dct["a"] = 2
-
-        with pytest.raises(TypeError):  # TODO: add msg
-            dct |= {"a": 3}
-
 
 def test_frozendict():
     dct = frozendict({"hello": "world"})

From 9e45e0294328b81300fd31044db2eba9127ab1fe Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 12:49:45 +0800
Subject: [PATCH 12/27] dont override |= to be consistent with frozenset

---
 src/monty/collections.py  | 15 ---------------
 tests/test_collections.py | 28 ++++++++--------------------
 2 files changed, 8 insertions(+), 35 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index d4ef8dbea..58c4eb7bd 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -56,7 +56,6 @@ class ControlledDict(collections.UserDict, ABC):
             - setter method: `__setitem__`
             - `setdefault`
             - `update`
-            - Merging dictionaries using `|=`
 
         - Deleting items:
             - `del dict[key]`
@@ -85,20 +84,6 @@ def __setitem__(self, key, value):
 
         super().__setitem__(key, value)
 
-    def __ior__(self, other):
-        """Handle the |= operator for dictionary updates."""
-        for key in other:
-            if key not in self.data and not self.allow_add:
-                raise TypeError(
-                    f"Cannot add new key using |= operator: {key!r}, because allow_add is set to False."
-                )
-            elif key in self.data and not self.allow_update:
-                raise TypeError(
-                    f"Cannot update key using |= operator: {key!r}, because allow_update is set to False."
-                )
-
-        return super().__ior__(other)
-
     def update(self, *args, **kwargs):
         """Forbid adding or updating keys based on allow_add and allow_update."""
         for key in dict(*args, **kwargs):
diff --git a/tests/test_collections.py b/tests/test_collections.py
index a071dee1a..0ef2ed313 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -31,14 +31,11 @@ def test_add_allowed(self):
         dct["b"] = 2
         assert dct["b"] == 2
 
-        dct |= {"c": 3}
-        assert dct["c"] == 3
+        dct.update(d=3)
+        assert dct["d"] == 3
 
-        dct.update(d=4)
-        assert dct["d"] == 4
-
-        dct.setdefault("e", 5)
-        assert dct["e"] == 5
+        dct.setdefault("e", 4)
+        assert dct["e"] == 4
 
     def test_add_disabled(self):
         dct = ControlledDict(a=1)
@@ -53,9 +50,6 @@ def test_add_disabled(self):
         with pytest.raises(TypeError, match="allow_add is set to False"):
             dct.setdefault("c", 2)
 
-        with pytest.raises(TypeError, match="allow_add is set to False"):
-            dct |= {"d": 2}
-
     def test_update_allowed(self):
         dct = ControlledDict(a=1)
         dct.allow_update = True
@@ -63,14 +57,11 @@ def test_update_allowed(self):
         dct["a"] = 2
         assert dct["a"] == 2
 
-        dct |= {"a": 3}
+        dct.update({"a": 3})
         assert dct["a"] == 3
 
-        dct.update({"a": 4})
-        assert dct["a"] == 4
-
-        dct.setdefault("a", 5)  # existing key
-        assert dct["a"] == 4
+        dct.setdefault("a", 4)  # existing key
+        assert dct["a"] == 3
 
     def test_update_disabled(self):
         dct = ControlledDict(a=1)
@@ -83,10 +74,7 @@ def test_update_disabled(self):
             dct.update({"a": 3})
 
         with pytest.raises(TypeError, match="allow_update is set to False"):
-            dct |= {"a": 4}
-
-        with pytest.raises(TypeError, match="allow_update is set to False"):
-            dct.setdefault("a", 5)
+            dct.setdefault("a", 4)
 
     def test_del_allowed(self):
         dct = ControlledDict(a=1, b=2, c=3, d=4)

From 74c4fd2492bd62591eb28eb2c2fc5dd8da0c17f2 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 13:52:25 +0800
Subject: [PATCH 13/27] fix controlled dict init

---
 src/monty/collections.py  | 37 +++++++++++++++++--------------------
 tests/test_collections.py | 36 ++++++++++++++++++++++++------------
 2 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 58c4eb7bd..a1b57876a 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -68,6 +68,18 @@ class ControlledDict(collections.UserDict, ABC):
     allow_del: ClassVar[bool] = True
     allow_update: ClassVar[bool] = True
 
+    def __init__(self, *args, **kwargs):
+        """Temporarily allow all add/update during initialization."""
+        original_allow_add = self.allow_add
+        original_allow_update = self.allow_update
+        try:
+            self.allow_add = True
+            self.allow_update = True
+            super().__init__(*args, **kwargs)
+        finally:
+            self.allow_add = original_allow_add
+            self.allow_update = original_allow_update
+
     # TODO: extract checkers
 
     # Overriding add/update operations
@@ -142,30 +154,15 @@ def clear(self):
         super().clear()
 
 
-class frozendict(dict):
+class frozendict(ControlledDict):
     """
     A dictionary that does not permit changes. The naming
-    violates PEP 8 to be consistent with the built-in "frozenset" naming.
+    violates PEP 8 to be consistent with the built-in `frozenset` naming.
     """
 
-    def __init__(self, *args, **kwargs) -> None:
-        """
-        Args:
-            args: Passthrough arguments for standard dict.
-            kwargs: Passthrough keyword arguments for standard dict.
-        """
-        dict.__init__(self, *args, **kwargs)
-
-    def __setitem__(self, key: Any, val: Any) -> None:
-        raise TypeError(f"{type(self).__name__} does not support item assignment")
-
-    def update(self, *args, **kwargs) -> None:
-        """
-        Args:
-            args: Passthrough arguments for standard dict.
-            kwargs: Passthrough keyword arguments for standard dict.
-        """
-        raise TypeError(f"Cannot update a {self.__class__.__name__}")
+    allow_add: ClassVar[bool] = False
+    allow_del: ClassVar[bool] = False
+    allow_update: ClassVar[bool] = False
 
 
 class Namespace(dict):  # TODO: this name is a bit confusing, deprecate it?
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 0ef2ed313..7c5ed5aa8 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from collections import UserDict
+
 import pytest
 
 from monty.collections import (
@@ -108,32 +110,42 @@ def test_del_disabled(self):
         with pytest.raises(TypeError, match="delete is disabled"):
             dct.clear()
 
+    def test_frozen_like(self):
+        """Make sure setter is allow at init time."""
+        ControlledDict.allow_add = False
+        ControlledDict.allow_update = False
+
+        dct = ControlledDict({"hello": "world"})
+        assert isinstance(dct, UserDict)
+        assert dct["hello"] == "world"
+
+        assert not dct.allow_add
+        assert not dct.allow_update
+
 
 def test_frozendict():
     dct = frozendict({"hello": "world"})
-    assert isinstance(dct, dict)
+    assert isinstance(dct, UserDict)
     assert dct["hello"] == "world"
 
+    assert not dct.allow_add
+    assert not dct.allow_update
+    assert not dct.allow_del
+
     # Test setter
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
-        dct["key"] == "val"
+    with pytest.raises(TypeError, match="allow_add is set to False"):
+        dct["key"] = "val"
 
     # Test update
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+    with pytest.raises(TypeError, match="allow_add is set to False"):
         dct.update(key="val")
 
-    # Test inplace-or (|=)
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
-        dct |= {"key": "val"}
-
-    # TODO: from this point we need a different error message
-
     # Test pop
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+    with pytest.raises(TypeError, match="delete is disabled"):
         dct.pop("key")
 
     # Test delete
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+    with pytest.raises(TypeError, match="delete is disabled"):
         del dct["key"]
 
 

From c2d3c20f836b63d8fc19bc29111034204091a355 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 13:57:59 +0800
Subject: [PATCH 14/27] more human readable error message

---
 src/monty/collections.py  | 50 ++++++++++-----------------------------
 tests/test_collections.py | 18 +++++++-------
 2 files changed, 22 insertions(+), 46 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index a1b57876a..9a09ad209 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -72,6 +72,7 @@ def __init__(self, *args, **kwargs):
         """Temporarily allow all add/update during initialization."""
         original_allow_add = self.allow_add
         original_allow_update = self.allow_update
+
         try:
             self.allow_add = True
             self.allow_update = True
@@ -80,19 +81,13 @@ def __init__(self, *args, **kwargs):
             self.allow_add = original_allow_add
             self.allow_update = original_allow_update
 
-    # TODO: extract checkers
-
-    # Overriding add/update operations
+    # Override add/update operations
     def __setitem__(self, key, value):
         """Forbid adding or updating keys based on allow_add and allow_update."""
         if key not in self.data and not self.allow_add:
-            raise TypeError(
-                f"Cannot add new key {key!r}, because allow_add is set to False."
-            )
+            raise TypeError(f"Cannot add new key {key!r}, because add is disabled.")
         elif key in self.data and not self.allow_update:
-            raise TypeError(
-                f"Cannot update key {key!r}, because allow_update is set to False."
-            )
+            raise TypeError(f"Cannot update key {key!r}, because update is disabled.")
 
         super().__setitem__(key, value)
 
@@ -101,11 +96,11 @@ def update(self, *args, **kwargs):
         for key in dict(*args, **kwargs):
             if key not in self.data and not self.allow_add:
                 raise TypeError(
-                    f"Cannot add new key {key!r} using update, because allow_add is set to False."
+                    f"Cannot add new key {key!r} using update, because add is disabled."
                 )
             elif key in self.data and not self.allow_update:
                 raise TypeError(
-                    f"Cannot update key {key!r} using update, because allow_update is set to False."
+                    f"Cannot update key {key!r} using update, because update is disabled."
                 )
 
         super().update(*args, **kwargs)
@@ -119,16 +114,16 @@ def setdefault(self, key, default=None):
         if key not in self.data:
             if not self.allow_add:
                 raise TypeError(
-                    f"Cannot add new key using setdefault: {key!r}, because allow_add is set to False."
+                    f"Cannot add new key using setdefault: {key!r}, because add is disabled."
                 )
         elif not self.allow_update:
             raise TypeError(
-                f"Cannot update key using setdefault: {key!r}, because allow_update is set to False."
+                f"Cannot update key using setdefault: {key!r}, because update is disabled."
             )
 
         return super().setdefault(key, default)
 
-    # Overriding delete operations
+    # Override delete operations
     def __delitem__(self, key):
         """Forbid deleting keys when self.allow_del is False."""
         if not self.allow_del:
@@ -165,31 +160,12 @@ class frozendict(ControlledDict):
     allow_update: ClassVar[bool] = False
 
 
-class Namespace(dict):  # TODO: this name is a bit confusing, deprecate it?
+class Namespace(ControlledDict):  # TODO: this name is a bit confusing, deprecate it?
     """A dictionary that does not permit changing its values."""
 
-    def __init__(self, *args, **kwargs) -> None:
-        """
-        Args:
-            args: Passthrough arguments for standard dict.
-            kwargs: Passthrough keyword arguments for standard dict.
-        """
-        self.update(*args, **kwargs)
-
-    def __setitem__(self, key: Any, val: Any) -> None:
-        if key in self:
-            raise TypeError(f"Cannot overwrite existing key: {key!s}")
-
-        dict.__setitem__(self, key, val)
-
-    def update(self, *args, **kwargs) -> None:
-        """
-        Args:
-            args: Passthrough arguments for standard dict.
-            kwargs: Passthrough keyword arguments for standard dict.
-        """
-        for k, v in dict(*args, **kwargs).items():
-            self[k] = v
+    allow_add: ClassVar[bool] = True
+    allow_del: ClassVar[bool] = True
+    allow_update: ClassVar[bool] = False
 
 
 class AttrDict(dict):
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 7c5ed5aa8..29652287c 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -43,13 +43,13 @@ def test_add_disabled(self):
         dct = ControlledDict(a=1)
         dct.allow_add = False
 
-        with pytest.raises(TypeError, match="allow_add is set to False"):
+        with pytest.raises(TypeError, match="add is disabled"):
             dct["b"] = 2
 
-        with pytest.raises(TypeError, match="allow_add is set to False"):
+        with pytest.raises(TypeError, match="add is disabled"):
             dct.update(b=2)
 
-        with pytest.raises(TypeError, match="allow_add is set to False"):
+        with pytest.raises(TypeError, match="add is disabled"):
             dct.setdefault("c", 2)
 
     def test_update_allowed(self):
@@ -69,13 +69,13 @@ def test_update_disabled(self):
         dct = ControlledDict(a=1)
         dct.allow_update = False
 
-        with pytest.raises(TypeError, match="allow_update is set to False"):
+        with pytest.raises(TypeError, match="update is disabled"):
             dct["a"] = 2
 
-        with pytest.raises(TypeError, match="allow_update is set to False"):
+        with pytest.raises(TypeError, match="update is disabled"):
             dct.update({"a": 3})
 
-        with pytest.raises(TypeError, match="allow_update is set to False"):
+        with pytest.raises(TypeError, match="update is disabled"):
             dct.setdefault("a", 4)
 
     def test_del_allowed(self):
@@ -133,11 +133,11 @@ def test_frozendict():
     assert not dct.allow_del
 
     # Test setter
-    with pytest.raises(TypeError, match="allow_add is set to False"):
+    with pytest.raises(TypeError, match="add is disabled"):
         dct["key"] = "val"
 
     # Test update
-    with pytest.raises(TypeError, match="allow_add is set to False"):
+    with pytest.raises(TypeError, match="add is disabled"):
         dct.update(key="val")
 
     # Test pop
@@ -151,7 +151,7 @@ def test_frozendict():
 
 def test_namespace_dict():
     dct = Namespace(key="val")
-    assert isinstance(dct, dict)
+    assert isinstance(dct, UserDict)
     dct["hello"] = "world"
     assert dct["key"] == "val"
 

From 7b96038c32ff6ec6afa5e24e12066babb5ca918c Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 14:21:37 +0800
Subject: [PATCH 15/27] enhance test for AttrDict

---
 src/monty/collections.py  | 51 ++++++---------------------------------
 tests/test_collections.py | 14 +++++++++--
 2 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 9a09ad209..c53ff8013 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -49,7 +49,7 @@ class ControlledDict(collections.UserDict, ABC):
         allow_update (bool): Whether existing keys can be updated.
 
     Configurable Operations:
-        This class allows controlling the following dictionary operations (refer to
+        This class allows controlling the following dict operations (refer to
         https://docs.python.org/3.13/library/stdtypes.html#mapping-types-dict for details):
 
         - Adding or updating items:
@@ -170,8 +170,8 @@ class Namespace(ControlledDict):  # TODO: this name is a bit confusing, deprecat
 
 class AttrDict(dict):
     """
-    Allows to access values as dct.key in addition
-    to the traditional way dct["key"]
+    Allow accessing values as `dct.key` in addition to
+    the traditional way `dct["key"]`.
 
     Examples:
         >>> dct = AttrDict(foo=1, bar=2)
@@ -181,53 +181,18 @@ class AttrDict(dict):
     """
 
     def __init__(self, *args, **kwargs) -> None:
-        """
-        Args:
-            args: Passthrough arguments for standard dict.
-            kwargs: Passthrough keyword arguments for standard dict.
-        """
-        super().__init__(*args, **kwargs)
+        super(AttrDict, self).__init__(*args, **kwargs)
         self.__dict__ = self
 
-    def copy(self) -> Self:
-        """
-        Returns:
-            Copy of AttrDict
-        """
-        newd = super().copy()
-        return self.__class__(**newd)
 
-
-class FrozenAttrDict(frozendict):
+class FrozenAttrDict(frozendict, AttrDict):  # TODO: fix inheritance
     """
     A dictionary that:
-        - Does not permit changes.
-        - Allows to access values as dct.key in addition
-          to the traditional way dct["key"]
+        - Does not permit changes (add/update/delete).
+        - Allow accessing values as `dct.key` in addition to
+            the traditional way `dct["key"]`.
     """
 
-    def __init__(self, *args, **kwargs) -> None:
-        """
-        Args:
-            args: Passthrough arguments for standard dict.
-            kwargs: Passthrough keyword arguments for standard dict.
-        """
-        super().__init__(*args, **kwargs)
-
-    def __getattribute__(self, name: str) -> Any:
-        try:
-            return super().__getattribute__(name)
-        except AttributeError:
-            try:
-                return self[name]
-            except KeyError as exc:
-                raise AttributeError(str(exc))
-
-    def __setattr__(self, name: str, value: Any) -> None:
-        raise TypeError(
-            f"You cannot modify attribute {name} of {self.__class__.__name__}"
-        )
-
 
 class MongoDict:
     """
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 29652287c..cb3bdfec9 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -111,7 +111,7 @@ def test_del_disabled(self):
             dct.clear()
 
     def test_frozen_like(self):
-        """Make sure setter is allow at init time."""
+        """Make sure add and update are allowed at init time."""
         ControlledDict.allow_add = False
         ControlledDict.allow_update = False
 
@@ -167,13 +167,23 @@ def test_namespace_dict():
 
 def test_attr_dict():
     dct = AttrDict(foo=1, bar=2)
+
+    # Test get attribute
     assert dct.bar == 2
     assert dct["foo"] is dct.foo
 
-    # Test accessing values
+    # Test key not found error
+    with pytest.raises(KeyError, match="no-such-key"):
+        dct["no-such-key"]
+
+    # Test setter
     dct.bar = "hello"
     assert dct["bar"] == "hello"
 
+    # Test delete
+    del dct.bar
+    assert "bar" not in dct
+
 
 def test_frozen_attrdict():
     dct = FrozenAttrDict({"hello": "world", 1: 2})

From a29d69f6c9c4db4b71c66ab5ae109f253e50288b Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 14:23:45 +0800
Subject: [PATCH 16/27] fix test_namespace_dict

---
 tests/test_collections.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/test_collections.py b/tests/test_collections.py
index cb3bdfec9..26d46777b 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -152,18 +152,18 @@ def test_frozendict():
 def test_namespace_dict():
     dct = Namespace(key="val")
     assert isinstance(dct, UserDict)
+
+    # Test setter
     dct["hello"] = "world"
     assert dct["key"] == "val"
 
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+    # Test update (not allowed)
+    with pytest.raises(TypeError, match="update is disabled"):
         dct["key"] = "val"
 
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
+    with pytest.raises(TypeError, match="update is disabled"):
         dct.update({"key": "val"})
 
-    with pytest.raises(TypeError, match="Cannot overwrite existing key"):
-        dct |= {"key": "val"}
-
 
 def test_attr_dict():
     dct = AttrDict(foo=1, bar=2)

From dc172c1d804529190eea37bc33fd6e46a4dc207f Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 14:56:16 +0800
Subject: [PATCH 17/27] disable delete for namespace dict

---
 src/monty/collections.py  | 6 +++---
 tests/test_collections.py | 6 +++++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index c53ff8013..c18cb665b 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -161,10 +161,10 @@ class frozendict(ControlledDict):
 
 
 class Namespace(ControlledDict):  # TODO: this name is a bit confusing, deprecate it?
-    """A dictionary that does not permit changing its values."""
+    """A dictionary that does not permit update/delete its values (but allows add)."""
 
     allow_add: ClassVar[bool] = True
-    allow_del: ClassVar[bool] = True
+    allow_del: ClassVar[bool] = False
     allow_update: ClassVar[bool] = False
 
 
@@ -185,7 +185,7 @@ def __init__(self, *args, **kwargs) -> None:
         self.__dict__ = self
 
 
-class FrozenAttrDict(frozendict, AttrDict):  # TODO: fix inheritance
+class FrozenAttrDict(frozendict):  # TODO: fix inheritance
     """
     A dictionary that:
         - Does not permit changes (add/update/delete).
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 26d46777b..c81f50777 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -164,6 +164,10 @@ def test_namespace_dict():
     with pytest.raises(TypeError, match="update is disabled"):
         dct.update({"key": "val"})
 
+    # Test delete (not allowed)
+    with pytest.raises(TypeError, match="delete is disabled"):
+        del dct["key"]
+
 
 def test_attr_dict():
     dct = AttrDict(foo=1, bar=2)
@@ -187,7 +191,7 @@ def test_attr_dict():
 
 def test_frozen_attrdict():
     dct = FrozenAttrDict({"hello": "world", 1: 2})
-    assert isinstance(dct, dict)
+    assert isinstance(dct, UserDict)
     assert dct["hello"] == "world"
     assert dct.hello == "world"
     assert dct["hello"] is dct.hello

From 86a15c1af82b5b1f21a38b6bc4d4a2a3114d11de Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 18:04:11 +0800
Subject: [PATCH 18/27] add method shadowing warning

---
 src/monty/collections.py  | 29 +++++++++++++++++++++++------
 tests/test_collections.py |  4 ++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index c18cb665b..d521f52ab 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -15,14 +15,13 @@
 from __future__ import annotations
 
 import collections
+import warnings
 from abc import ABC
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
     from typing import Any, ClassVar, Iterable
 
-    from typing_extensions import Self
-
 
 def tree() -> collections.defaultdict:
     """
@@ -170,27 +169,45 @@ class Namespace(ControlledDict):  # TODO: this name is a bit confusing, deprecat
 
 class AttrDict(dict):
     """
-    Allow accessing values as `dct.key` in addition to
-    the traditional way `dct["key"]`.
+    Allow accessing values as `dct.key` in addition to the traditional way `dct["key"]`.
 
     Examples:
         >>> dct = AttrDict(foo=1, bar=2)
         >>> assert dct["foo"] is dct.foo
         >>> dct.bar = "hello"
-        >>> assert dct.bar == "hello"
+
+    Warnings:
+        When shadowing dict methods, e.g.:
+            >>> dct = AttrDict(update="value")
+            >>> dct.update()  # TypeError (the `update` method is overwritten)
+
+    References:
+        https://stackoverflow.com/a/14620633/24021108
     """
 
     def __init__(self, *args, **kwargs) -> None:
         super(AttrDict, self).__init__(*args, **kwargs)
         self.__dict__ = self
 
+    def __setitem__(self, key, value) -> None:
+        """Check if the key shadows dict method."""
+        if key in dir(dict):
+            warnings.warn(
+                f"'{key=}' shadows dict method. This may lead to unexpected behavior.",
+                UserWarning,
+                stacklevel=2,
+            )
+        super().__setitem__(key, value)
 
-class FrozenAttrDict(frozendict):  # TODO: fix inheritance
+
+class FrozenAttrDict(frozendict):
     """
     A dictionary that:
         - Does not permit changes (add/update/delete).
         - Allow accessing values as `dct.key` in addition to
             the traditional way `dct["key"]`.
+
+    TODO:
     """
 
 
diff --git a/tests/test_collections.py b/tests/test_collections.py
index c81f50777..30992303b 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -188,6 +188,10 @@ def test_attr_dict():
     del dct.bar
     assert "bar" not in dct
 
+    # Test builtin dict method shadowing
+    with pytest.warns(UserWarning, match="shadows dict method"):
+        dct["update"] = "value"
+
 
 def test_frozen_attrdict():
     dct = FrozenAttrDict({"hello": "world", 1: 2})

From 374d4624411b7f4f0ba05f662d41eae8abd996a0 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 18:31:30 +0800
Subject: [PATCH 19/27] clean up FrozenAttrDict

---
 src/monty/collections.py  | 39 +++++++++++++++++++++++++++++++++++--
 tests/test_collections.py | 41 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index d521f52ab..65b429ae9 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -200,16 +200,51 @@ def __setitem__(self, key, value) -> None:
         super().__setitem__(key, value)
 
 
-class FrozenAttrDict(frozendict):
+class FrozenAttrDict(AttrDict):
     """
     A dictionary that:
         - Does not permit changes (add/update/delete).
         - Allow accessing values as `dct.key` in addition to
             the traditional way `dct["key"]`.
 
-    TODO:
+    TODO: I didn't manage to inherit from `frozendict` which would trigger a RecursionError.
     """
 
+    def __init__(self, *args, **kwargs) -> None:
+        """Allow setting only during initialization."""
+        object.__setattr__(self, "_initialized", False)
+        super().__init__(*args, **kwargs)
+        object.__setattr__(self, "_initialized", True)
+
+    def __setattr__(self, key, value):
+        """Allow setting only during initialization."""
+        if getattr(self, "_initialized", True) and key != "_initialized":
+            raise TypeError(
+                f"{self.__class__.__name__} does not support item assignment."
+            )
+        super().__setattr__(key, value)
+
+    def __setitem__(self, key, value):
+        raise TypeError(f"{self.__class__.__name__} does not support item assignment.")
+
+    def update(self, *args, **kwargs):
+        raise TypeError(f"{self.__class__.__name__} does not support update.")
+
+    def clear(self):
+        raise TypeError(f"{self.__class__.__name__} does not support clear.")
+
+    def pop(self, key, *args):
+        raise TypeError(f"{self.__class__.__name__} does not support pop.")
+
+    def popitem(self):
+        raise TypeError(f"{self.__class__.__name__} does not support popitem.")
+
+    def __delitem__(self, key):
+        raise TypeError(f"{self.__class__.__name__} does not support deletion.")
+
+    def __delattr__(self, key):
+        raise TypeError(f"{self.__class__.__name__} does not support deletion.")
+
 
 class MongoDict:
     """
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 30992303b..7b30c7039 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -195,23 +195,54 @@ def test_attr_dict():
 
 def test_frozen_attrdict():
     dct = FrozenAttrDict({"hello": "world", 1: 2})
-    assert isinstance(dct, UserDict)
+    assert isinstance(dct, dict)
+
+    # Test get value
     assert dct["hello"] == "world"
     assert dct.hello == "world"
     assert dct["hello"] is dct.hello
 
     # Test adding item
-    with pytest.raises(TypeError, match="You cannot modify attribute"):
+    with pytest.raises(
+        TypeError, match="FrozenAttrDict does not support item assignment"
+    ):
         dct["foo"] = "bar"
-    with pytest.raises(TypeError, match="You cannot modify attribute"):
+    with pytest.raises(
+        TypeError, match="FrozenAttrDict does not support item assignment"
+    ):
         dct.foo = "bar"
 
     # Test modifying existing item
-    with pytest.raises(TypeError, match="You cannot modify attribute"):
+    with pytest.raises(
+        TypeError, match="FrozenAttrDict does not support item assignment"
+    ):
         dct.hello = "new"
-    with pytest.raises(TypeError, match="You cannot modify attribute"):
+    with pytest.raises(
+        TypeError, match="FrozenAttrDict does not support item assignment"
+    ):
         dct["hello"] = "new"
 
+    # Test update
+    with pytest.raises(TypeError, match="FrozenAttrDict does not support update"):
+        dct.update({"hello": "world"})
+
+    # Test pop
+    with pytest.raises(TypeError, match="FrozenAttrDict does not support pop"):
+        dct.pop("hello")
+
+    with pytest.raises(TypeError, match="FrozenAttrDict does not support popitem"):
+        dct.popitem()
+
+    # Test delete
+    with pytest.raises(TypeError, match="FrozenAttrDict does not support deletion"):
+        del dct["hello"]
+
+    with pytest.raises(TypeError, match="FrozenAttrDict does not support deletion"):
+        del dct.hello
+
+    with pytest.raises(TypeError, match="FrozenAttrDict does not support clear"):
+        dct.clear()
+
 
 def test_mongo_dict():
     m_dct = MongoDict({"a": {"b": 1}, "x": 2})

From 56c65ce28f326e32f039428eaea171268b8eb804 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 18:58:30 +0800
Subject: [PATCH 20/27] simplify FrozenAttrDict

---
 src/monty/collections.py  | 102 ++++++++++++++++----------------------
 tests/test_collections.py |  48 ++++++++----------
 2 files changed, 64 insertions(+), 86 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 65b429ae9..ed4298b16 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -43,9 +43,9 @@ class ControlledDict(collections.UserDict, ABC):
     A base dictionary class with configurable mutability.
 
     Attributes:
-        allow_add (bool): Whether new keys can be added.
-        allow_del (bool): Whether existing keys can be deleted.
-        allow_update (bool): Whether existing keys can be updated.
+        _allow_add (bool): Whether new keys can be added.
+        _allow_del (bool): Whether existing keys can be deleted.
+        _allow_update (bool): Whether existing keys can be updated.
 
     Configurable Operations:
         This class allows controlling the following dict operations (refer to
@@ -63,29 +63,26 @@ class ControlledDict(collections.UserDict, ABC):
             - `clear()`
     """
 
-    allow_add: ClassVar[bool] = True
-    allow_del: ClassVar[bool] = True
-    allow_update: ClassVar[bool] = True
+    _allow_add: ClassVar[bool] = True
+    _allow_del: ClassVar[bool] = True
+    _allow_update: ClassVar[bool] = True
 
     def __init__(self, *args, **kwargs):
-        """Temporarily allow all add/update during initialization."""
-        original_allow_add = self.allow_add
-        original_allow_update = self.allow_update
+        """Temporarily allow add during initialization."""
+        original_allow_add = self._allow_add
 
         try:
-            self.allow_add = True
-            self.allow_update = True
+            self._allow_add = True
             super().__init__(*args, **kwargs)
         finally:
-            self.allow_add = original_allow_add
-            self.allow_update = original_allow_update
+            self._allow_add = original_allow_add
 
     # Override add/update operations
     def __setitem__(self, key, value):
         """Forbid adding or updating keys based on allow_add and allow_update."""
-        if key not in self.data and not self.allow_add:
+        if key not in self.data and not self._allow_add:
             raise TypeError(f"Cannot add new key {key!r}, because add is disabled.")
-        elif key in self.data and not self.allow_update:
+        elif key in self.data and not self._allow_update:
             raise TypeError(f"Cannot update key {key!r}, because update is disabled.")
 
         super().__setitem__(key, value)
@@ -93,11 +90,11 @@ def __setitem__(self, key, value):
     def update(self, *args, **kwargs):
         """Forbid adding or updating keys based on allow_add and allow_update."""
         for key in dict(*args, **kwargs):
-            if key not in self.data and not self.allow_add:
+            if key not in self.data and not self._allow_add:
                 raise TypeError(
                     f"Cannot add new key {key!r} using update, because add is disabled."
                 )
-            elif key in self.data and not self.allow_update:
+            elif key in self.data and not self._allow_update:
                 raise TypeError(
                     f"Cannot update key {key!r} using update, because update is disabled."
                 )
@@ -111,11 +108,11 @@ def setdefault(self, key, default=None):
             new default value is the same as current value for efficiency.
         """
         if key not in self.data:
-            if not self.allow_add:
+            if not self._allow_add:
                 raise TypeError(
                     f"Cannot add new key using setdefault: {key!r}, because add is disabled."
                 )
-        elif not self.allow_update:
+        elif not self._allow_update:
             raise TypeError(
                 f"Cannot update key using setdefault: {key!r}, because update is disabled."
             )
@@ -125,25 +122,25 @@ def setdefault(self, key, default=None):
     # Override delete operations
     def __delitem__(self, key):
         """Forbid deleting keys when self.allow_del is False."""
-        if not self.allow_del:
+        if not self._allow_del:
             raise TypeError(f"Cannot delete key {key!r}, because delete is disabled.")
         super().__delitem__(key)
 
     def pop(self, key, *args):
         """Forbid popping keys when self.allow_del is False."""
-        if not self.allow_del:
+        if not self._allow_del:
             raise TypeError(f"Cannot pop key {key!r}, because delete is disabled.")
         return super().pop(key, *args)
 
     def popitem(self):
         """Forbid popping the last item when self.allow_del is False."""
-        if not self.allow_del:
+        if not self._allow_del:
             raise TypeError("Cannot pop item, because delete is disabled.")
         return super().popitem()
 
     def clear(self):
         """Forbid clearing the dictionary when self.allow_del is False."""
-        if not self.allow_del:
+        if not self._allow_del:
             raise TypeError("Cannot clear dictionary, because delete is disabled.")
         super().clear()
 
@@ -154,17 +151,17 @@ class frozendict(ControlledDict):
     violates PEP 8 to be consistent with the built-in `frozenset` naming.
     """
 
-    allow_add: ClassVar[bool] = False
-    allow_del: ClassVar[bool] = False
-    allow_update: ClassVar[bool] = False
+    _allow_add: ClassVar[bool] = False
+    _allow_del: ClassVar[bool] = False
+    _allow_update: ClassVar[bool] = False
 
 
 class Namespace(ControlledDict):  # TODO: this name is a bit confusing, deprecate it?
     """A dictionary that does not permit update/delete its values (but allows add)."""
 
-    allow_add: ClassVar[bool] = True
-    allow_del: ClassVar[bool] = False
-    allow_update: ClassVar[bool] = False
+    _allow_add: ClassVar[bool] = True
+    _allow_del: ClassVar[bool] = False
+    _allow_update: ClassVar[bool] = False
 
 
 class AttrDict(dict):
@@ -200,50 +197,35 @@ def __setitem__(self, key, value) -> None:
         super().__setitem__(key, value)
 
 
-class FrozenAttrDict(AttrDict):
+class FrozenAttrDict(frozendict):
     """
     A dictionary that:
         - Does not permit changes (add/update/delete).
         - Allow accessing values as `dct.key` in addition to
             the traditional way `dct["key"]`.
-
-    TODO: I didn't manage to inherit from `frozendict` which would trigger a RecursionError.
     """
 
-    def __init__(self, *args, **kwargs) -> None:
-        """Allow setting only during initialization."""
-        object.__setattr__(self, "_initialized", False)
+    def __init__(self, *args, **kwargs):
+        """Allow add during init, as __setattr__ is called unlike frozendict."""
+        self._allow_add = True
         super().__init__(*args, **kwargs)
-        object.__setattr__(self, "_initialized", True)
+        self._allow_add = False
+
+    def __getattribute__(self, name):
+        try:
+            return super().__getattribute__(name)
+        except AttributeError:
+            return self[name]
 
-    def __setattr__(self, key, value):
-        """Allow setting only during initialization."""
-        if getattr(self, "_initialized", True) and key != "_initialized":
+    def __setattr__(self, name, value):
+        if not self._allow_add and name != "_allow_add":
             raise TypeError(
                 f"{self.__class__.__name__} does not support item assignment."
             )
-        super().__setattr__(key, value)
-
-    def __setitem__(self, key, value):
-        raise TypeError(f"{self.__class__.__name__} does not support item assignment.")
-
-    def update(self, *args, **kwargs):
-        raise TypeError(f"{self.__class__.__name__} does not support update.")
-
-    def clear(self):
-        raise TypeError(f"{self.__class__.__name__} does not support clear.")
-
-    def pop(self, key, *args):
-        raise TypeError(f"{self.__class__.__name__} does not support pop.")
-
-    def popitem(self):
-        raise TypeError(f"{self.__class__.__name__} does not support popitem.")
-
-    def __delitem__(self, key):
-        raise TypeError(f"{self.__class__.__name__} does not support deletion.")
+        super().__setattr__(name, value)
 
-    def __delattr__(self, key):
-        raise TypeError(f"{self.__class__.__name__} does not support deletion.")
+    def __delattr__(self, name):
+        raise TypeError(f"{self.__class__.__name__} does not support item deletion.")
 
 
 class MongoDict:
diff --git a/tests/test_collections.py b/tests/test_collections.py
index 7b30c7039..87d216134 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -28,7 +28,7 @@ def test_tree():
 class TestControlledDict:
     def test_add_allowed(self):
         dct = ControlledDict(a=1)
-        dct.allow_add = True
+        dct._allow_add = True
 
         dct["b"] = 2
         assert dct["b"] == 2
@@ -41,7 +41,7 @@ def test_add_allowed(self):
 
     def test_add_disabled(self):
         dct = ControlledDict(a=1)
-        dct.allow_add = False
+        dct._allow_add = False
 
         with pytest.raises(TypeError, match="add is disabled"):
             dct["b"] = 2
@@ -54,7 +54,7 @@ def test_add_disabled(self):
 
     def test_update_allowed(self):
         dct = ControlledDict(a=1)
-        dct.allow_update = True
+        dct._allow_update = True
 
         dct["a"] = 2
         assert dct["a"] == 2
@@ -67,7 +67,7 @@ def test_update_allowed(self):
 
     def test_update_disabled(self):
         dct = ControlledDict(a=1)
-        dct.allow_update = False
+        dct._allow_update = False
 
         with pytest.raises(TypeError, match="update is disabled"):
             dct["a"] = 2
@@ -80,7 +80,7 @@ def test_update_disabled(self):
 
     def test_del_allowed(self):
         dct = ControlledDict(a=1, b=2, c=3, d=4)
-        dct.allow_del = True
+        dct._allow_del = True
 
         del dct["a"]
         assert "a" not in dct
@@ -96,7 +96,7 @@ def test_del_allowed(self):
 
     def test_del_disabled(self):
         dct = ControlledDict(a=1)
-        dct.allow_del = False
+        dct._allow_del = False
 
         with pytest.raises(TypeError, match="delete is disabled"):
             del dct["a"]
@@ -112,15 +112,15 @@ def test_del_disabled(self):
 
     def test_frozen_like(self):
         """Make sure add and update are allowed at init time."""
-        ControlledDict.allow_add = False
-        ControlledDict.allow_update = False
+        ControlledDict._allow_add = False
+        ControlledDict._allow_update = False
 
         dct = ControlledDict({"hello": "world"})
         assert isinstance(dct, UserDict)
         assert dct["hello"] == "world"
 
-        assert not dct.allow_add
-        assert not dct.allow_update
+        assert not dct._allow_add
+        assert not dct._allow_update
 
 
 def test_frozendict():
@@ -128,9 +128,9 @@ def test_frozendict():
     assert isinstance(dct, UserDict)
     assert dct["hello"] == "world"
 
-    assert not dct.allow_add
-    assert not dct.allow_update
-    assert not dct.allow_del
+    assert not dct._allow_add
+    assert not dct._allow_update
+    assert not dct._allow_del
 
     # Test setter
     with pytest.raises(TypeError, match="add is disabled"):
@@ -195,7 +195,7 @@ def test_attr_dict():
 
 def test_frozen_attrdict():
     dct = FrozenAttrDict({"hello": "world", 1: 2})
-    assert isinstance(dct, dict)
+    assert isinstance(dct, UserDict)
 
     # Test get value
     assert dct["hello"] == "world"
@@ -203,9 +203,7 @@ def test_frozen_attrdict():
     assert dct["hello"] is dct.hello
 
     # Test adding item
-    with pytest.raises(
-        TypeError, match="FrozenAttrDict does not support item assignment"
-    ):
+    with pytest.raises(TypeError, match="add is disabled"):
         dct["foo"] = "bar"
     with pytest.raises(
         TypeError, match="FrozenAttrDict does not support item assignment"
@@ -217,30 +215,28 @@ def test_frozen_attrdict():
         TypeError, match="FrozenAttrDict does not support item assignment"
     ):
         dct.hello = "new"
-    with pytest.raises(
-        TypeError, match="FrozenAttrDict does not support item assignment"
-    ):
+    with pytest.raises(TypeError, match="update is disabled"):
         dct["hello"] = "new"
 
     # Test update
-    with pytest.raises(TypeError, match="FrozenAttrDict does not support update"):
+    with pytest.raises(TypeError, match="update is disabled"):
         dct.update({"hello": "world"})
 
     # Test pop
-    with pytest.raises(TypeError, match="FrozenAttrDict does not support pop"):
+    with pytest.raises(TypeError, match="delete is disabled"):
         dct.pop("hello")
 
-    with pytest.raises(TypeError, match="FrozenAttrDict does not support popitem"):
+    with pytest.raises(TypeError, match="delete is disabled"):
         dct.popitem()
 
     # Test delete
-    with pytest.raises(TypeError, match="FrozenAttrDict does not support deletion"):
+    with pytest.raises(TypeError, match="delete is disabled"):
         del dct["hello"]
 
-    with pytest.raises(TypeError, match="FrozenAttrDict does not support deletion"):
+    with pytest.raises(TypeError, match="does not support item deletion"):
         del dct.hello
 
-    with pytest.raises(TypeError, match="FrozenAttrDict does not support clear"):
+    with pytest.raises(TypeError, match="delete is disabled"):
         dct.clear()
 
 

From 777da53a0364cfbd30aadd04ccd882b7c665e62f Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 19:01:32 +0800
Subject: [PATCH 21/27] clarify class var name

---
 src/monty/collections.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index ed4298b16..84966dc58 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -79,7 +79,7 @@ def __init__(self, *args, **kwargs):
 
     # Override add/update operations
     def __setitem__(self, key, value):
-        """Forbid adding or updating keys based on allow_add and allow_update."""
+        """Forbid adding or updating keys based on _allow_add and _allow_update."""
         if key not in self.data and not self._allow_add:
             raise TypeError(f"Cannot add new key {key!r}, because add is disabled.")
         elif key in self.data and not self._allow_update:
@@ -88,7 +88,7 @@ def __setitem__(self, key, value):
         super().__setitem__(key, value)
 
     def update(self, *args, **kwargs):
-        """Forbid adding or updating keys based on allow_add and allow_update."""
+        """Forbid adding or updating keys based on _allow_add and _allow_update."""
         for key in dict(*args, **kwargs):
             if key not in self.data and not self._allow_add:
                 raise TypeError(
@@ -102,9 +102,9 @@ def update(self, *args, **kwargs):
         super().update(*args, **kwargs)
 
     def setdefault(self, key, default=None):
-        """Forbid adding or updating keys based on allow_add and allow_update.
+        """Forbid adding or updating keys based on _allow_add and _allow_update.
 
-        Note: if not allow_update, this method would NOT check whether the
+        Note: if not _allow_update, this method would NOT check whether the
             new default value is the same as current value for efficiency.
         """
         if key not in self.data:
@@ -121,25 +121,25 @@ def setdefault(self, key, default=None):
 
     # Override delete operations
     def __delitem__(self, key):
-        """Forbid deleting keys when self.allow_del is False."""
+        """Forbid deleting keys when self._allow_del is False."""
         if not self._allow_del:
             raise TypeError(f"Cannot delete key {key!r}, because delete is disabled.")
         super().__delitem__(key)
 
     def pop(self, key, *args):
-        """Forbid popping keys when self.allow_del is False."""
+        """Forbid popping keys when self._allow_del is False."""
         if not self._allow_del:
             raise TypeError(f"Cannot pop key {key!r}, because delete is disabled.")
         return super().pop(key, *args)
 
     def popitem(self):
-        """Forbid popping the last item when self.allow_del is False."""
+        """Forbid popping the last item when self._allow_del is False."""
         if not self._allow_del:
             raise TypeError("Cannot pop item, because delete is disabled.")
         return super().popitem()
 
     def clear(self):
-        """Forbid clearing the dictionary when self.allow_del is False."""
+        """Forbid clearing the dictionary when self._allow_del is False."""
         if not self._allow_del:
             raise TypeError("Cannot clear dictionary, because delete is disabled.")
         super().clear()
@@ -201,7 +201,7 @@ class FrozenAttrDict(frozendict):
     """
     A dictionary that:
         - Does not permit changes (add/update/delete).
-        - Allow accessing values as `dct.key` in addition to
+        - Allows accessing values as `dct.key` in addition to
             the traditional way `dct["key"]`.
     """
 

From a356b1279af5c1d581c80f86fe41990fd4b7ab5d Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 19:09:18 +0800
Subject: [PATCH 22/27] fix mypy but not 100% sure

---
 src/monty/collections.py | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 84966dc58..51bbd5b36 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -20,7 +20,7 @@
 from typing import TYPE_CHECKING
 
 if TYPE_CHECKING:
-    from typing import Any, ClassVar, Iterable
+    from typing import Any, Iterable
 
 
 def tree() -> collections.defaultdict:
@@ -63,11 +63,11 @@ class ControlledDict(collections.UserDict, ABC):
             - `clear()`
     """
 
-    _allow_add: ClassVar[bool] = True
-    _allow_del: ClassVar[bool] = True
-    _allow_update: ClassVar[bool] = True
+    _allow_add: bool = True
+    _allow_del: bool = True
+    _allow_update: bool = True
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         """Temporarily allow add during initialization."""
         original_allow_add = self._allow_add
 
@@ -78,7 +78,7 @@ def __init__(self, *args, **kwargs):
             self._allow_add = original_allow_add
 
     # Override add/update operations
-    def __setitem__(self, key, value):
+    def __setitem__(self, key, value) -> None:
         """Forbid adding or updating keys based on _allow_add and _allow_update."""
         if key not in self.data and not self._allow_add:
             raise TypeError(f"Cannot add new key {key!r}, because add is disabled.")
@@ -87,7 +87,7 @@ def __setitem__(self, key, value):
 
         super().__setitem__(key, value)
 
-    def update(self, *args, **kwargs):
+    def update(self, *args, **kwargs) -> None:
         """Forbid adding or updating keys based on _allow_add and _allow_update."""
         for key in dict(*args, **kwargs):
             if key not in self.data and not self._allow_add:
@@ -101,7 +101,7 @@ def update(self, *args, **kwargs):
 
         super().update(*args, **kwargs)
 
-    def setdefault(self, key, default=None):
+    def setdefault(self, key, default=None) -> None:
         """Forbid adding or updating keys based on _allow_add and _allow_update.
 
         Note: if not _allow_update, this method would NOT check whether the
@@ -120,7 +120,7 @@ def setdefault(self, key, default=None):
         return super().setdefault(key, default)
 
     # Override delete operations
-    def __delitem__(self, key):
+    def __delitem__(self, key) -> None:
         """Forbid deleting keys when self._allow_del is False."""
         if not self._allow_del:
             raise TypeError(f"Cannot delete key {key!r}, because delete is disabled.")
@@ -138,7 +138,7 @@ def popitem(self):
             raise TypeError("Cannot pop item, because delete is disabled.")
         return super().popitem()
 
-    def clear(self):
+    def clear(self) -> None:
         """Forbid clearing the dictionary when self._allow_del is False."""
         if not self._allow_del:
             raise TypeError("Cannot clear dictionary, because delete is disabled.")
@@ -151,17 +151,17 @@ class frozendict(ControlledDict):
     violates PEP 8 to be consistent with the built-in `frozenset` naming.
     """
 
-    _allow_add: ClassVar[bool] = False
-    _allow_del: ClassVar[bool] = False
-    _allow_update: ClassVar[bool] = False
+    _allow_add: bool = False
+    _allow_del: bool = False
+    _allow_update: bool = False
 
 
 class Namespace(ControlledDict):  # TODO: this name is a bit confusing, deprecate it?
     """A dictionary that does not permit update/delete its values (but allows add)."""
 
-    _allow_add: ClassVar[bool] = True
-    _allow_del: ClassVar[bool] = False
-    _allow_update: ClassVar[bool] = False
+    _allow_add: bool = True
+    _allow_del: bool = False
+    _allow_update: bool = False
 
 
 class AttrDict(dict):
@@ -205,26 +205,26 @@ class FrozenAttrDict(frozendict):
             the traditional way `dct["key"]`.
     """
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         """Allow add during init, as __setattr__ is called unlike frozendict."""
         self._allow_add = True
         super().__init__(*args, **kwargs)
         self._allow_add = False
 
-    def __getattribute__(self, name):
+    def __getattribute__(self, name: str) -> Any:
         try:
             return super().__getattribute__(name)
         except AttributeError:
             return self[name]
 
-    def __setattr__(self, name, value):
+    def __setattr__(self, name: str, value: Any) -> None:
         if not self._allow_add and name != "_allow_add":
             raise TypeError(
                 f"{self.__class__.__name__} does not support item assignment."
             )
         super().__setattr__(name, value)
 
-    def __delattr__(self, name):
+    def __delattr__(self, name: str) -> None:
         raise TypeError(f"{self.__class__.__name__} does not support item deletion.")
 
 

From 52405447e3803555959ed060910985a1a7b4dc71 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 19:14:23 +0800
Subject: [PATCH 23/27] remove todo tag

---
 src/monty/collections.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 51bbd5b36..fda851351 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -156,7 +156,7 @@ class frozendict(ControlledDict):
     _allow_update: bool = False
 
 
-class Namespace(ControlledDict):  # TODO: this name is a bit confusing, deprecate it?
+class Namespace(ControlledDict):
     """A dictionary that does not permit update/delete its values (but allows add)."""
 
     _allow_add: bool = True

From 04339d84a68561083382743dcc360a8b28be9cae Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 19:30:09 +0800
Subject: [PATCH 24/27] use object attribute to avoid recursion

---
 src/monty/collections.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index fda851351..19da696f0 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -213,7 +213,7 @@ def __init__(self, *args, **kwargs) -> None:
 
     def __getattribute__(self, name: str) -> Any:
         try:
-            return super().__getattribute__(name)
+            return object.__getattribute__(self, name)
         except AttributeError:
             return self[name]
 

From 638d1fea56624c4c1b06b76ef820ae731877ba83 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 19:34:31 +0800
Subject: [PATCH 25/27] fix setdefault return type

---
 src/monty/collections.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index 19da696f0..b6e9a4008 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -101,7 +101,7 @@ def update(self, *args, **kwargs) -> None:
 
         super().update(*args, **kwargs)
 
-    def setdefault(self, key, default=None) -> None:
+    def setdefault(self, key, default=None) -> Any:
         """Forbid adding or updating keys based on _allow_add and _allow_update.
 
         Note: if not _allow_update, this method would NOT check whether the

From d575d598b848f8223977442d58b2007b1787ca0c Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 19:42:13 +0800
Subject: [PATCH 26/27] fix typo in docstring

---
 src/monty/collections.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/monty/collections.py b/src/monty/collections.py
index b6e9a4008..154a9c863 100644
--- a/src/monty/collections.py
+++ b/src/monty/collections.py
@@ -306,7 +306,7 @@ def dict2namedtuple(*args, **kwargs) -> tuple:
         >>> assert tpl.foo == 1 and tpl.bar == "hello"
 
         >>> tpl = dict2namedtuple([("foo", 1), ("bar", "hello")])
-        >>> assert tpl[0] is tpl.foo and t[1] is tpl.bar
+        >>> assert tpl[0] is tpl.foo and tpl[1] is tpl.bar
 
     Warnings:
         - The order of the items in the namedtuple is not deterministic if

From a1d0a137aaa349be0ee9dcd4a158e94856e10a96 Mon Sep 17 00:00:00 2001
From: "Haoyu (Daniel)" <yanghaoyu97@outlook.com>
Date: Wed, 27 Nov 2024 19:45:39 +0800
Subject: [PATCH 27/27] clean up test

---
 tests/test_collections.py | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/tests/test_collections.py b/tests/test_collections.py
index 87d216134..118a08a87 100644
--- a/tests/test_collections.py
+++ b/tests/test_collections.py
@@ -197,24 +197,26 @@ def test_frozen_attrdict():
     dct = FrozenAttrDict({"hello": "world", 1: 2})
     assert isinstance(dct, UserDict)
 
+    # Test attribute-like operations
+    with pytest.raises(TypeError, match="does not support item assignment"):
+        dct.foo = "bar"
+
+    with pytest.raises(TypeError, match="does not support item assignment"):
+        dct.hello = "new"
+
+    with pytest.raises(TypeError, match="does not support item deletion"):
+        del dct.hello
+
     # Test get value
     assert dct["hello"] == "world"
     assert dct.hello == "world"
-    assert dct["hello"] is dct.hello
+    assert dct["hello"] is dct.hello  # identity check
 
     # Test adding item
     with pytest.raises(TypeError, match="add is disabled"):
         dct["foo"] = "bar"
-    with pytest.raises(
-        TypeError, match="FrozenAttrDict does not support item assignment"
-    ):
-        dct.foo = "bar"
 
     # Test modifying existing item
-    with pytest.raises(
-        TypeError, match="FrozenAttrDict does not support item assignment"
-    ):
-        dct.hello = "new"
     with pytest.raises(TypeError, match="update is disabled"):
         dct["hello"] = "new"
 
@@ -233,9 +235,6 @@ def test_frozen_attrdict():
     with pytest.raises(TypeError, match="delete is disabled"):
         del dct["hello"]
 
-    with pytest.raises(TypeError, match="does not support item deletion"):
-        del dct.hello
-
     with pytest.raises(TypeError, match="delete is disabled"):
         dct.clear()