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

Fix contact size for 128c-4s6mm6cm-15um-26um-sl.yml #103

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

edeno
Copy link
Collaborator

@edeno edeno commented Feb 4, 2025

Fixes #102

@edeno edeno requested review from samuelbray32 and acomrie February 4, 2025 19:49
Copy link

@acomrie acomrie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo fix in geom looks good (I'm not aware of the contact_size field being used downstream, I think the longer device name, plus the ids and relative x y z coordinates within the file are used directly for spikesorting last i checked)

Copy link
Collaborator

@samuelbray32 samuelbray32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values end up in Probe.Electrode as contact_size. I haven't tested spikesorting, but I would suspect that it pulls the info from the probe device in the nwbfile (came up with Jen's data last summer for the relative positions of electrodes).

Also, should change it here too

samuelbray32 added a commit to samuelbray32/franklabnwb that referenced this pull request Feb 6, 2025
Matching with [this PR](LorenFrankLab/trodes_to_nwb#103 (review)) in `trodes_to_nwb`
@edeno
Copy link
Collaborator Author

edeno commented Feb 6, 2025

@samuelbray32 by the way, do you know what's going on with the tests here?

@samuelbray32
Copy link
Collaborator

Oh I hadn't caught that. No that looks like something outside of us has changed in the last 4 months. I'll poke around

@samuelbray32
Copy link
Collaborator

Tests pass locally, let me see if can find the offending package

@edeno
Copy link
Collaborator Author

edeno commented Feb 6, 2025

Looking at the errors, I suspect the cause is the latest numpy version.

@samuelbray32
Copy link
Collaborator

Yeah, I can create and solve the failed test locally by changing numpy version. Suspect it's in the np.fromfile() function

@samuelbray32
Copy link
Collaborator

Solved the ones related to the wrong shape in position. Does stem from some change in np.fromfile.

Still working on the int8 overflow

@samuelbray32
Copy link
Collaborator

Working on updating for numpy on #104

Copy link

codecov bot commented Feb 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.34%. Comparing base (a0a3b67) to head (e951e83).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #103   +/-   ##
=======================================
  Coverage   90.34%   90.34%           
=======================================
  Files          25       25           
  Lines        2433     2433           
=======================================
  Hits         2198     2198           
  Misses        235      235           

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

@samuelbray32 samuelbray32 merged commit 8db322d into main Feb 7, 2025
10 checks passed
@edeno edeno deleted the fix-contact-size branch February 7, 2025 19:44
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.

typo in yml file
3 participants