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

Question: why is grid_spacing not a field in ImagingPlane? #1885

Closed
h-mayorquin opened this issue Apr 8, 2024 · 2 comments · Fixed by #1892
Closed

Question: why is grid_spacing not a field in ImagingPlane? #1885

h-mayorquin opened this issue Apr 8, 2024 · 2 comments · Fixed by #1892
Assignees
Labels
category: bug errors in the code or code behavior priority: medium non-critical problem and/or affecting only a small set of NWB users
Milestone

Comments

@h-mayorquin
Copy link
Contributor

I would like to understand the logic of why some attributes of the class are fields and others are not. For the ImagingPlane we are missing grid_spacing and origin_coords

pynwb/src/pynwb/ophys.py

Lines 33 to 46 in 2259bed

class ImagingPlane(NWBContainer):
"""An imaging plane and its metadata."""
__nwbfields__ = ({'name': 'optical_channel', 'child': True},
'description',
'device',
'excitation_lambda',
'imaging_rate',
'indicator',
'location',
'manifold',
'conversion',
'unit',
'reference_frame')

This is related to the html representation of the object. We use the fields to generate the html representation (that is very handy for exploring) and therefore some attributes are not displayed because they are not fields which I think is a big miss.

https://github.com/hdmf-dev/hdmf/blob/d85d0cbc36d2e0fdb25e8fbea14d58ba7bf24a40/src/hdmf/container.py#L685-L693

@oruebel
Copy link
Contributor

oruebel commented Apr 9, 2024

@rly thoughts?

@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 NWB users labels Apr 11, 2024
@rly rly added this to the Next Release milestone Apr 11, 2024
@stephprince stephprince modified the milestones: 2.7.0, 2.8.0 Apr 11, 2024
@h-mayorquin
Copy link
Contributor Author

For future reference I discussed this with @rly in person. This is a bug and attributes here mentioned should be fields. #1892 should solve this.

It is still unclear to me if all the arguments of a constructor in pynwb should be fields. Is that the case?

@rly rly closed this as completed in #1892 May 2, 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 NWB users
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants