Skip to content

[Blocked - Waiting for upstream changes] Ephys tool from obi-one #377

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

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jankrepl
Copy link
Collaborator

@jankrepl jankrepl commented Jun 23, 2025

Closes #375

For reviewers

  • The endpoint will give you an empty result for human traces - the obi-one maintainers said they would make error messages nicer in the next iteration. So it is better to only filter for traces that are not human. Note that I stumbled upon an issue in entitycore when trying to run that query (Species filter leads to an error entitycore#250) so maybe the easiest way to test this PR is to just use this trace id f3948959-9d55-4fef-840d-b6b7e36e92e1
  • I did some renaming - feel free to protest

TODO

@jankrepl jankrepl marked this pull request as ready for review June 24, 2025 12:58
Copy link
Collaborator

@WonderPG WonderPG left a comment

Choose a reason for hiding this comment

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

Looks good to me, I will play a bit with it still but I don't see what can go wrong on our side.
One thing I dislike about this new endpoint in obi-one is that it does the exact same thing as our code did before, in fact it is our old code. Some things were not good in this old code imo, for instance defaulting to every single feature and every single stimulus if nothing is provided.

Also I expect some changes to come since imo they messed up by not exposing the actual parameters in the API. I suggest we keep this one open for a bit since changes will need to occur once they fix it.

@jankrepl jankrepl changed the title Ephys tool from obi-one [Blocked - Waiting for upstream changes] Ephys tool from obi-one Jun 25, 2025
@jankrepl
Copy link
Collaborator Author

Also I expect some changes to come since imo they messed up by not exposing the actual parameters in the API. I suggest we keep this one open for a bit since changes will need to occur once they fix it.

Fair. I changed the ticket title and let's wait for obione maintainers to make the endpoint more versatile.

@jankrepl jankrepl marked this pull request as draft June 25, 2025 08:58
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.

Tool for electrophysiologyrecording-metrics/{trace_id} from obi-one
2 participants