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]: Custom class generator does not account for fields refined from parent type #1101

Open
rly opened this issue Apr 21, 2024 · 1 comment
Assignees
Labels
category: bug errors in the code or code behavior priority: low alternative solution already working and/or relevant to only specific user(s) topic: extension issues related to extensions or dynamic class generation
Milestone

Comments

@rly
Copy link
Contributor

rly commented Apr 21, 2024

What happened?

In PyNWB, the TimeSeries spec has a dataset spec with name "data", dtype "numeric", many possible shapes, and a docstring. If an extension type ExtracellularSeries extends TimeSeries and has a dataset spec with name "data", dtype "float", one possible shape, and a different docstring, then when generating ExtracellularSeries.__init__, the "data" keyword argument from TimeSeries.__init__ is used instead of a new one being generated with the refined properties.

Same for the attributes of the "data" dataset spec like "unit", "conversion", etc.

I don't remember why we even use the docstring arguments from TimeSeries.__init__ rather than always regenerating them from the spec. It may have to do with fixed value and fixed name specs and how those are not part of added as keyword arguments to __init__.

Steps to Reproduce

See description above.

Traceback

No response

Operating System

macOS

Python Executable

Conda

Python Version

3.12

Package Versions

No response

@rly rly self-assigned this Apr 21, 2024
@rly rly added category: bug errors in the code or code behavior topic: extension issues related to extensions or dynamic class generation labels Apr 21, 2024
@rly rly added this to the 3.14.0 milestone Apr 21, 2024
@mavaylon1
Copy link
Contributor

That is a weird decision to use the docstring inputs from TimeSeries. Thanks for assigning labels and a milestone.

@rly rly added the priority: low alternative solution already working and/or relevant to only specific user(s) label Jun 27, 2024
@rly rly modified the milestones: 3.14.2, Future Jun 27, 2024
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: low alternative solution already working and/or relevant to only specific user(s) topic: extension issues related to extensions or dynamic class generation
Projects
None yet
Development

No branches or pull requests

2 participants