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

propsed fix for spikeglx streamer failing on SDSC because of wrong uuid in file name #905

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

grg2rsr
Copy link

@grg2rsr grg2rsr commented Jan 13, 2025

previously, the Streamer was instantiated with a absolute path of the meta file (also not ideal in the context of the Streamers constructors call signature, where the first argument is named sglx_file and a dedicated argument meta_file exists. link). While this looks like a bug, it is recovered there in the following lines under normal (=non-SDSC) usage as this line

meta_file = meta_file or _get_companion_file(sglx_file, ".meta")

recoveres the correct file paths.

However, this breaks on the SDSC as the file paths an uuid, and when the corresponding file path for the meta for the bin (or vice versa) are constructed, the uuid is wrong.

The proposed patch checks if the .meta filename contains an uuid, and if so, finds the correct uuid for the .cbin file, constructs absolute paths for both. Either way, uuid or not, the Streamer is now instantiated with both paths for .meta and .bin (or .cbin).

@grg2rsr grg2rsr requested a review from oliche January 13, 2025 19:26
@grg2rsr grg2rsr marked this pull request as draft January 13, 2025 19:39
@grg2rsr
Copy link
Author

grg2rsr commented Jan 13, 2025

thinking more about this, this could also be patched in a different way, where the uuid completing functionality is added to the _get_companion_file() function in ibl-neuropixel/spikeglx, with a remove_uuid=True as a default kwarg, which is then turned to False either here and passed through the Streamer constructor. This could cover future occurrences of this bug.

Copy link
Member

@oliche oliche left a comment

Choose a reason for hiding this comment

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

SDSC is probably the only place on Earth where we do not need the Streaming functionality ! 😄

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