-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Context
- I find it a little strange that Reader is now in api.py. Does it make sense to add an internal "base.py" or "core.py" module that contains Reader? This could in the future contain also the base Writer class? That way we would avoid the cyclical dependency where reader.py and api.py import from each other.
Indeed this was initially forced by the cyclic dependency, but in fact I can see that "api.py" is really playing the role here of "base.py" or "core.py". Most of the times I think we should recommend just importing directly from
swc.aeon.ioand maybe we could make "api.py" private or move it into__init__.py.I did not do this for backwards compatibility purposes (note
Readeris still available as a simbol in "reader.py" since it is imported as a base class there), but I'm happy to discuss restructuring all these namespaces as I've mentioned before.@lochhh Let me know your thoughts on this, they are very helpful.
In my mind, api contains the end-user-facing functions (e.g. for achieving what is in this notebook) - a casual user is not expected to create a Reader but rather access this through an existing Device, whereas core/base is dev-facing (e.g. devs can build on Readers, add Devices, etc.). But this is getting out of scope for the current PR, which is good to go!
Originally posted by @lochhh in #60 (comment)
Proposal
Introduce base.py or core.py (similar to #44) , i.e. the Reader class would remain in reader.py, Reader subclasses in core.py