Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harmonize memory constructors/signatures #2565

Open
dgw opened this issue Nov 18, 2023 · 0 comments
Open

Harmonize memory constructors/signatures #2565

dgw opened this issue Nov 18, 2023 · 0 comments
Labels
Housekeeping Code cleanup, removal of deprecated stuff, etc.
Milestone

Comments

@dgw
Copy link
Member

dgw commented Nov 18, 2023

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.)

@dgw dgw added the Housekeeping Code cleanup, removal of deprecated stuff, etc. label Nov 18, 2023
@dgw dgw added this to the 8.1.0 milestone Nov 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping Code cleanup, removal of deprecated stuff, etc.
Projects
None yet
Development

No branches or pull requests

1 participant