Repository navigation
Add GECI sensor parameter fallbacks and update gecis.csv - #73
kruthi-munnangi wants to merge 2 commits into
Conversation
|
Found 10 changed notebooks. Review the changes at https://app.gitnotebooks.com/siplab-gt/cleo/pull/73 |
kjohnsen
left a comment
kjohnsen
left a comment
There was a problem hiding this comment.
Still a ways to go, but a good start!
There was a problem hiding this comment.
these changes are not from your changes, but rather differences with master branch. Make sure to merge master into this branch to avoid stuff like this
There was a problem hiding this comment.
same here. your GECI code shouldn't affect any of this ephys code, so it's a red flag that there are changes here. Fix by git merge origin master
There was a problem hiding this comment.
It's annoying to manually maintain the list of imports as opposed to a star import, but its necessary to make the package user-friendly for IDE auto-completion, etc.
https://share.gemini.google/emE6YjITX02y
There was a problem hiding this comment.
Make sure names are the way they are in literature (e.g., GCaMP3 instead of gcamp3). There is code that automatically turns those into names that are good for Python/Brian, called in _create_geci_fn()
There was a problem hiding this comment.
what is the citation column used for? Why are they all Dana2019? There are at least 3 different papers we could cite here...where the GECI was originally presented, where we get params from, and where we get the excitation spectrum from
| def test_light_dependent_geci_is_correct_type(): | ||
| """LightDependentGECI should be successfully instantiated with arguments""" | ||
| sensor = LightDependentGECI( | ||
| name="test_sensor", |
There was a problem hiding this comment.
It's more useful to test the API that we intend for the user, e.g., jgcamp7f(light_dependent=True) rather than manually constructing these objects
There was a problem hiding this comment.
These tests are a good start, but don't test getting meaningful signals out of a simulation. I'd look at test_geci() in test_sensors.py for a test that actually ensures there's a signal. And add the important things we expect, like dFF that depends on the wavelength used, or higher SNR for higher power/lower sampling rate/smaller image.
2 participants
No description provided.