get_class fails with recursive definition

[t-b]

I'm trying to add some get_class defaults for ndx-MIES as requested by @rly in https://github.com/nwb-extensions/staged-extensions/pull/15.

But it fails on the recursive type definition:

https://github.com/t-b/ndx-MIES/blob/5e91258312b835aab87942d336081cec37192f86/src/spec/create_extension_spec.py#L55-L67

Versions: pynwb 1.4.0 hdmf 2.2.0

Reproducable example:

git clone https://github.com/t-b/ndx-MIES
git checkout pynwb-issue-recursive-def
pip install -e .
cd src/pynwb/tests
pytest .

Output:

================================================= test session starts =================================================
platform win32 -- Python 3.7.4, pytest-5.2.1, py-1.8.0, pluggy-0.13.0
rootdir: E:\projekte\ndx-mies
plugins: arraydiff-0.3, doctestplus-0.4.0, openfiles-0.4.0, remotedata-0.3.2
collected 0 items / 1 errors

======================================================= ERRORS ========================================================
___________________________________ ERROR collecting src/pynwb/tests/test_basics.py ___________________________________
test_basics.py:2: in <module>
    import ndx_mies
..\ndx_mies\__init__.py:30: in <module>
    StimulusSetReferencedFolder = get_class('StimulusSetReferencedFolder', 'ndx-mies')
C:\users\thomas\Anaconda3\lib\site-packages\hdmf\utils.py:565: in func_call
    return func(**pargs)
C:\users\thomas\Anaconda3\lib\site-packages\pynwb\__init__.py:187: in get_class
    return __TYPE_MAP.get_container_cls(namespace, neurodata_type)
C:\users\thomas\Anaconda3\lib\site-packages\hdmf\utils.py:561: in func_call
    return func(args[0], **pargs)
C:\users\thomas\Anaconda3\lib\site-packages\hdmf\build\manager.py:592: in get_container_cls
    self.__resolve_child_container_classes(spec, namespace)
C:\users\thomas\Anaconda3\lib\site-packages\hdmf\build\manager.py:635: in __resolve_child_container_classes
    self.get_container_cls(namespace, child_spec.data_type_inc)
C:\users\thomas\Anaconda3\lib\site-packages\hdmf\utils.py:561: in func_call
    return func(args[0], **pargs)
E   RecursionError: maximum recursion depth exceeded in comparison
!!! Recursion detected (same locals & position)
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
================================================== 1 error in 5.18s ===================================================

Just to be clear, there is data released with this extension already, so I can not change the extension schema in an incompatible way.

[bendichter]

thanks @t-b. Unfortunately, I think this is a fundamental limitation of our current implementation of get_class. Would be interesting to discuss a solution to this for get_class, but for now you'll have to specify the API manually in these cases.

[oruebel]

BTW, eliminating nested definitions in the schema should not change it in an incompatible way as it typically just means placing the type definitions at the top level and then using neurodata_type_inc to include them in the proper places.

[t-b]

@bendichter I'm not sure I'll find time to write custom code for that.

@oruebel This is not the usual nested spec problem.

From the posted link above

    stimset_referenced_folder = NWBGroupSpec(doc='Folder',
                                             neurodata_type_def='StimulusSetReferencedFolder',
                                             datasets=[...],
                                             groups=[NWBGroupSpec(doc='Nested Folder',
                                                                  neurodata_type_inc='StimulusSetReferencedFolder',
                                                                  quantity='*')
                                                          ],
                                             )

which means that the group spec contains itself recursively. And get_class chokes on that. I don't see a way to rewrite that.

[rly]

If this were a simple recursion where type A can contain one instance of type A, then I have a solution that involves some changes in HDMF. However, this is more complicated in that type A can contain multiple instances of type A, which results in the automatic generation of a MultiContainerInterface subclass. I have tried a couple different approaches and it looks like changing get_class to handle this use case will be pretty involved. I will have to get back to this in a few days.

This also reveals another bug where it is not possible to generate classes that are MultiContainerInterface where the base class is a class that has fields beyond just a name.

[t-b]

@rly Is it possible to write a custom class for StimulusSetReferencedFolder?