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

Camstim ephys #144

Merged
merged 119 commits into from
Sep 11, 2024
Merged

Camstim ephys #144

merged 119 commits into from
Sep 11, 2024

Conversation

rcpeene
Copy link
Collaborator

@rcpeene rcpeene commented Sep 10, 2024

I don't know why this wasn't caught elsewhere, or if it was, but it appears the latest schema has different objects so the camstim ephys class wouldn't work. Perhaps I'm mislead, but these changes appeared to be compatible.

rcpeene and others added 30 commits May 3, 2024 14:58
@@ -68,7 +67,7 @@ def __init__(self, session_id: str, json_settings: dict) -> None:
self.recording_dir = npc_ephys.get_single_oebin_path(
session_inst.lims_path
).parent
except FileNotFoundError:
except:
Copy link
Contributor

Choose a reason for hiding this comment

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

why was the FileNotFoundError removed? Using a bare except without specifying an exception type will catch all exceptions, including ones that you might not intend to handle (like KeyboardInterrupt, SystemExit, or MemoryError).

This can make debugging more difficult so I'd recommend putting FileNotFoundError back or if you need to catch multiple types of errors you can do a tuple

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There was another error received here but I forget what it was. I should have written it down. I guess we can revert and if it shows up again I can commit again

return stim_name


def get_stimulus_type(stimulus):
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

fill in docstrings

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like all functions here have docstrings already. am I missing something?

@mekhlakapoor mekhlakapoor merged commit ea983a5 into dev Sep 11, 2024
3 checks passed
@mekhlakapoor mekhlakapoor deleted the camstim-ephys branch September 11, 2024 23:39
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.

3 participants