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

Support read with classes using REGISTER_SUBCLASS_WITH_TYPENAME #115

Open
oruebel opened this issue Dec 20, 2024 · 0 comments
Open

Support read with classes using REGISTER_SUBCLASS_WITH_TYPENAME #115

oruebel opened this issue Dec 20, 2024 · 0 comments
Labels
category: enhancement proposed enhancements or new features priority: medium non-critical problem and/or affecting only a small set of users

Comments

@oruebel
Copy link
Contributor

oruebel commented Dec 20, 2024

This is a follow-up issue to #91, #85 . Currently for classes that use REGISTER_SUBCLASS_WITH_TYPENAME can be used to create a new NWB file, but are not automatically used when reading from a file. This is because these classes do not have a corresponding neurodata_type in the schema, and the logic for detecting the type to use is purely based on neurodata_type right now. See:

* A special version of the ``REGISTER_SUBCLASS`` macro, called ``REGISTER_SUBCLASS_WITH_TYPENAME``,
* allows setting the typename explicitly as a third argument. This is for the **special case**
* where we want to implement a class for a modified type that does not have its
* own `neurodata_type` in the NWB schema. An example is `ElectrodesTable` in NWB <v2.7, which
* did not have an assigned `neurodata_type`, but was implemented as a regular
* `DynamicTable`. To allow us to define a class `ElectrodeTable` to help with writing the table
* we can then use ``REGISTER_SUBCLASS_WITH_TYPENAME(ElectrodeTable, "core", "DynamicTable")``
* in the `ElectrodesTable` class. This ensures that the `neurodata_type` attribute is set
* correctly to `DynamicTable` on write instead of `ElectrodesTable`. However, on read this
* will by default not use the `ElectrodesTable` class but the regular `DynamicTable` class
* since that is what the schema is indicating to use. In the registry, the class will still
* be registered under the ``core::ElectrodesTable`` key, but with "DynamicTable" as the
* typename value and the `ElectrodesTable.getTypeName` automatic override returning
* the indicated typename instead of the classname.

Possible solutions:

  • ElectrodeTable (which is currently the main case for this), is planned to get a neurodata_type assigned at some point. We can wait for that schema change and update ElectrodeTable then. However, this does not fix the more general user-case/problem.
  • We could: 1) extend REGISTER_SUBCLASS_WITH_TYPENAME to support specifying a fixed path, 2) update RegisteredType::create to also consider the path when deciding on the class to use, 3) update the registry in RegisteredType to support storing and looking up types with an additional optional path
@oruebel oruebel added category: enhancement proposed enhancements or new features priority: medium non-critical problem and/or affecting only a small set of users labels Dec 20, 2024
@oruebel oruebel mentioned this issue Dec 20, 2024
68 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: enhancement proposed enhancements or new features priority: medium non-critical problem and/or affecting only a small set of users
Projects
None yet
Development

No branches or pull requests

1 participant