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]: Resolving spec inheritance fails when attribute has same name as existing attr on another object #1121

Closed
rly opened this issue Jun 1, 2024 · 1 comment · Fixed by #1122
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 topic: extension issues related to extensions or dynamic class generation

Comments

@rly
Copy link
Contributor

rly commented Jun 1, 2024

What happened?

From catalystneuro/ndx-microscopy#11 (comment)

The relevant spec:

  - neurodata_type_def: ImagingSpace
    neurodata_type_inc: LabMetaData  # Would prefer basic NWBContainer
    doc: Metadata about the region of physical space that imaging data was recorded from.
    datasets:
    - name: origin_coordinates
      dtype: float64
      dims:
        - - x, y, z
      shape:
        - - 3
      doc: Physical location in stereotactic coordinates for the first element of the grid.
        See reference_frame to determine what the coordinates are relative to (e.g., bregma).
      quantity: '?'
      attributes:
        - name: unit
          dtype: text
          default_value: micrometers
          doc: Measurement units for origin coordinates. The default value is 'micrometers'.

  - neurodata_type_def: PlanarImagingSpace
    neurodata_type_inc: ImagingSpace
    doc: Metadata about the 2-dimensional slice of physical space that imaging data was recorded from.
    datasets:
    - name: grid_spacing
      dtype: float64
      dims:
        - - x, y
      shape:
        - - 2
      doc: Amount of space between pixels in the specified unit.
        Specify 'z' only when imaging volume is a regular grid; otherwise only specify 'x' and 'y'.
        See origin_coordinates to determine where the grid begins.
      quantity: '?'
      attributes:
        - name: unit
          dtype: text
          default_value: micrometers
          doc: Measurement units for grid spacing. The default value is 'micrometers'.

After doing some digging, I found that because ImagingSpec.origin_coordinates has an attribute unit and PlanarImagingSpace.grid_spacing also has an attribute unit, that when determining whether PlanarImagingSpace.grid_spacing.unit is inherited or not, HDMF says yes it is inherited because when ImagingSpec.origin_coordinates is inherited and it has an attribute with the same name, unit. That is not correct.

I have a fix.

Steps to Reproduce

Install ndx-microscopy linked above. Uncomment `unit` attribute spec on the `PlanarImagingSpace` spec. Instantiate a `PlanarImagingSpace` with `grid_spacing__unit="unit"`

Traceback

No response

Operating System

macOS

Python Executable

Conda

Python Version

3.12

Package Versions

No response

@rly rly added category: bug errors in the code or code behavior priority: medium non-critical problem and/or affecting only a small set of users topic: extension issues related to extensions or dynamic class generation labels Jun 1, 2024
@rly rly self-assigned this Jun 1, 2024
@mavaylon1
Copy link
Contributor

Thanks for diving into this! @rly

@rly rly closed this as completed in #1122 Jun 5, 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: medium non-critical problem and/or affecting only a small set of users topic: extension issues related to extensions or dynamic class generation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants