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

Storage: Use tinybase.storage.Storable directly #162

Closed
liamhuber opened this issue Jan 12, 2024 · 1 comment
Closed

Storage: Use tinybase.storage.Storable directly #162

liamhuber opened this issue Jan 12, 2024 · 1 comment

Comments

@liamhuber
Copy link
Member

Right now #160 uses its own to_/from_storage methods to interact with the storage. Rather, let's inherit from Storage and use the _re/store functionality directly.

This depends on #161 so that including Storable in the MRO no longer causes such a costly import.

This should be quite straightforward for Node, but may be trickier for DataChannel, which requires a Node instance (the owning parent) at instantiation. This causes circular storage if we do it naively.

Related, for Macro we need to take care that children are already created at instantiation, so we may need to replace these with the loaded objects in the _restore. This would be computationally inefficient, but would allow people to modify macros and then save them.... (Probably macros need to get some internal flag for whether or not they've been tampered with, so that when it comes to logging them in a database we know about "true" instances of that macro vs "modified" instances......)

In any case, @pmrv and I discussed that it may be useful to modify the signature of to_object and restore. E.g. I am thinking it could also accept **kwargs that get passed to the class-specific _restore -- perhaps ChannelData._restore(storage, version, node), and in Node._restore we could do something like storage["inputs"][label].to_object(node=self) to pass the extra required info at re-store time without ever having in

@liamhuber liamhuber mentioned this issue Jan 20, 2024
7 tasks
@liamhuber
Copy link
Member Author

My current thinking is to instead modify the infrastructure to use __get(set)state__ instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant