Skip to content

Conversation

@h-mayorquin
Copy link
Contributor

@h-mayorquin h-mayorquin commented Dec 10, 2025

As discussed with @rly @stephprince and @rly

Take a look at the format.

image image

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Does the PR clearly describe the problem and the solution?
  • Have you reviewed our Contributing Guide?
  • Does the PR use "Fix #XXX" notation to tell GitHub to close the relevant issue numbered XXX when the PR is merged?

@codecov
Copy link

codecov bot commented Dec 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.44%. Comparing base (a90f8ed) to head (91780c9).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #1355   +/-   ##
=======================================
  Coverage   92.44%   92.44%           
=======================================
  Files          42       42           
  Lines        9742     9747    +5     
  Branches     1978     1979    +1     
=======================================
+ Hits         9006     9011    +5     
  Misses        462      462           
  Partials      274      274           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rly
Copy link
Contributor

rly commented Dec 12, 2025

@h-mayorquin could you please resolve the conflicts?

Why exclude top-level fields? I think it's useful, e.g., for the Units and Electrodes tables.

The html repr is really about how the file is represented in memory in Python, as opposed to on-disk (for example, the "general" group is not shown). In almost all cases, the objects in the repr correspond to PyNWB/HDMF classes, and those match the neurodata type. In rare cases, they might deviate, e.g., when a class with a different name from the data type is mapped to the data type. Could we just use the class name instead of the data type name?

@h-mayorquin
Copy link
Contributor Author

Why exclude top-level fields? I think it's useful, e.g., for the Units and Electrodes tables.

I was looking at the trials and epochs that are also inside an intervals accessor at the top level and inferring wrongly that there all the top level things are just general containers (e.g. acquisition, processing, intervals, etc). But then, we have units and electrodes that are not like this so I guess let's just always display this.

@h-mayorquin
Copy link
Contributor Author

@rly I modified the code to match the suggestions.

@rly rly merged commit 79b6d03 into hdmf-dev:dev Dec 13, 2025
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants