You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.)
The text was updated successfully, but these errors were encountered:
With #2525, a
SopelIdentifierMemory
can be initialized with the contents of anotherSopelIdentifierMemory
or a plaindict
, or a sequence of key-value tuples. With #2552, aSopelIdentifierMemory
can be instantiated without having to worry about theidentifier_factory
kwarg (thebot
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 inbot
doesn't. Plus, all of thetools.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, plusSopelMemoryWithDefault
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.)The text was updated successfully, but these errors were encountered: