SopelIdentifierMemory pop() does not cast

[half-duplex]

Description

SopelIdentifierMemory automatically casts strings to Identifiers when accessing contents with mem["foo"], but pop() doesn't seem to.

Reproduction steps

>>> from sopel.tools import Identifier, SopelIdentifierMemory
>>> mem = SopelIdentifierMemory()
>>> mem["Foo"] = "bar"
>>> mem["Foo"]
'bar'
>>> mem.pop("Foo")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'Foo'
>>> mem.pop(Identifier("Foo"))
'bar'

Note the capital F in "Foo" - hashing Identifier("foo") and "foo" seems to match, but not Identifier("Foo") and "Foo".

Expected behavior

pop() performs the same cast as mem["Foo"] and works.

Sopel version

271b306

Installation method

pip install

Python version

3.11.5

[dgw]

Follow-up note from IRC: The whole dict API should be checked when resolving this.

[dgw]

Went down the doc page testing each operation listed under dict and found the following:

• Working as expected
  • d['key'] = 'foo'
  • list(d)
  • len(d)
  • d['key']
  • d['KeY']
  • del d['key']
  • 'key' in d
  • 'KeY' in d
  • iter(d)
  • SopelIdentifierMemory.fromkeys(('foo', 'bar', 'baz'), True)
    Surprised me that this actually does create an object of the correct type, but the C implementation appears to create a new object of the class it was called on.
    • I wonder if this should be overridden with a pure Python implementation anyway, to guard against implementation differences between CPython and PyPy/IronPython/whatever else
  • d.items()/d.keys()/d.values()
    Created iv/kv/vv references to these views, made changes to d, and ensured the views all updated.
  • d.pop('KeY') (on draft PR's branch)
  • d.pop('NotPresent', 'default') (on draft PR's branch)
  • d.popitem()
  • reversed(d)
  • d.setdefault('key', 'default')
  • d.update(another_identifiermemory)
  • d | another_identifiermemory (new feature in py3.9)
  • d |= another_identifiermemory (new feature in py3.9)
• Wrong behavior or unexpected error
  • del d['KeY']
  • d.clear() (appears to work, but the object becomes a regular dict and loses the Identifier magic)
  • d.copy()
    Made a copy m and added another item to it, verifying that the new item did not appear in the original d.
    However, the new object m is a dict, not a SopelIdentifierMemory.
  • d.get('KeY')
    Returns the default passed to .get(), not key's value.
  • d.pop('NotInDict')
    On draft PR's branch, returns None but should raise KeyError.
  • d.setdefault('KeY', 'default')
    With 'key' in d, returns 'default' & adds 'KeY' to d as a regular string, not Identifier.
  • d.update(a_plain_dict)
    If d has Identifier('thing1') as a key, and a_plain_dict has 'ThinG1' (a regular string), they will not be merged correctly and d will now also have a regular string key called 'ThinG1'.
  • d | a_plain_dict
    Same problem as d.update(a_plain_dict).
  • d |= a_plain_dict
    Same problem as d.update(a_plain_dict) and d | a_plain_dict.
  • d == a_plain_dict
    If a_plain_dict has the same key/value pairs but the key capitalization doesn't match Identifier's canonical (lowercase) internal representation, the equality check returns False.
  • result = SopelIdentifierMemory({'KeY': 'foo'})
result is of type SopelIdentifierMemory, but without the magic. Existing keys are not converted to Identifier, and 'key' in result doesn't work as expected for that type.
    In fact, even 'KeY' in result returns False for the example above, and result['KeY'] raises KeyError`. Even basic functionality of the resulting fake identifier memory is completely broken.

…I might not actually finish #2525 today, lol. That's a lot of stuff to figure out how to handle, then write unit tests for. 😅