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

[Bug]: Auto-generating MultiContainerInterface subclasses #959

Closed
3 tasks done
mavaylon1 opened this issue Oct 3, 2023 · 2 comments
Closed
3 tasks done

[Bug]: Auto-generating MultiContainerInterface subclasses #959

mavaylon1 opened this issue Oct 3, 2023 · 2 comments
Assignees
Labels
category: bug errors in the code or code behavior priority: medium non-critical problem and/or affecting only a small set of users

Comments

@mavaylon1
Copy link
Contributor

mavaylon1 commented Oct 3, 2023

What happened?

First alerted in #948, there is a bug from auto-generating class that inherits from MultiContainerInterface. In this example, PlaneExtension extends PlaneSegmentation, which inherits from MultiContainerInterface (you can see this in the schema with reference_images having quantity *. With extensions, NWBInspector does not see the python class definitions and will autogenerate the class from the schema. In this example, PlaneExtension is auto-generated. PlaneExtension inherits from both MultiContainerInterface and from PlaneSegmentation, which is a DynamicTable.

Within DynamicTable is a pre-init method _gather_columns that uses the base classes in a conditional to create pre-defined columns. When autogenerating a class, the order of these classes are determined in lines 377-385 of classgenerator.py

if issubclass(MultiContainerInterface, bases[0]):
    # if bases[0] is Container or another superclass of MCI, then make sure MCI goes first
    # otherwise, MRO is ambiguous
    bases.insert(0, MultiContainerInterface)
else:
    # bases[0] is not a subclass of MCI and not a superclass of MCI. place that class first
    # before MCI. that class __init__ should call super().__init__ which will call the
    # MCI init
    bases.insert(1, MultiContainerInterface)

Since MultiContainerInterface is not a sub-class of DynamicTable (bases[0]), we insert at index 1.

Within gather_columns, we check

if (len(bases) and 'DynamicTable' in globals() and issubclass(bases[-1], Container)
                and bases[-1].__columns__ is not cls.__columns__):

Such that bases[-1] (the last base in this case would be MultiContainerInterface) is assumed to be a DynamicTable and checks for columns. This returns the error:
AttributeError: type object 'MultiContainerInterface' has no attribute '__columns__'

To fix this, the search for DynamicTable should be agnostic to the order of the bases.

Steps to Reproduce

# Run the extension or un-register a class.

Traceback

No response

Operating System

macOS

Python Executable

Python

Python Version

3.10

Package Versions

No response

Code of Conduct

@mavaylon1
Copy link
Contributor Author

I'll target this before the end of October.

@mavaylon1 mavaylon1 added category: bug errors in the code or code behavior priority: medium non-critical problem and/or affecting only a small set of users labels Oct 18, 2023
@rly
Copy link
Contributor

rly commented Oct 30, 2023

@mavaylon1 Could you tackle this issue this week?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: bug errors in the code or code behavior priority: medium non-critical problem and/or affecting only a small set of users
Projects
None yet
Development

No branches or pull requests

2 participants