sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
951 stars 405 forks source link

Harmonize memory constructors/signatures #2565

Open dgw opened 7 months ago

dgw commented 7 months ago

With #2525, a SopelIdentifierMemory can be initialized with the contents of another SopelIdentifierMemory or a plain dict, or a sequence of key-value tuples. With #2552, a SopelIdentifierMemory can be instantiated without having to worry about the identifier_factory kwarg (the bot has a helper that handles this for you).

However, this leaves things in a bit of an inconsistent state: SopelIdentifierMemory explicitly takes optional starting data, but its helper in bot doesn't. Plus, all of the tools.memories types still use variadic *args signatures that don't reflect the actual usage semantics.

Consensus in #2552 (and associated IRC discussion) was that we can sort this out in 8.1.0. Fixing the constructor-parameter signatures & handling won't actually break anything, as it's already a TypeError to pass multiple positional arguments to all three memory types, plus SopelMemoryWithDefault has a different signature (collections.defaultdict eats the first parameter as its default factory).

All we want to do is make the constructor signatures in our code match the underlying object—maybe without the "kwargs-as-key-value-pairs" part, for simplicity—and probably add test cases for their behaviors. (tools.memories is at 98% coverage, but most of that is just from the objects being used in various places and doesn't specifically verify behavior.)