-
Notifications
You must be signed in to change notification settings - Fork 0
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
Sue test #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, following Python's PEP8 convention, could you change all function and variable names from camelCase to snake_case? ChatGPT/copilot can easily help :)
"""Example.""" | ||
data_dir = Path(os.path.dirname(__file__)).parent.parent | ||
nwbfile = os.path.join( | ||
data_dir, "tests\\data\\689514_2024-02-01_18-06-43.nwb" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change all \\
s in the path to /
so that they work for both Windows and Linux? I get an error when I run this in Code Ocean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the tip! Also changed other cases in the code.
leftLicks = nwb.acquisition["left_lick_time"].timestamps[:] | ||
rightLicks = nwb.acquisition["right_lick_time"].timestamps[:] | ||
allLicks = np.sort(np.concatenate((rightLicks, leftLicks))) | ||
sortInd = np.argsort(np.concatenate((rightLicks, leftLicks))) | ||
allLicksID = np.concatenate( | ||
(np.ones_like(rightLicks), np.zeros_like(leftLicks)) | ||
) | ||
allLicksID = allLicksID[sortInd] | ||
allLickDiffs = np.diff(allLicks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we should separate the code that extracts data from nwb and the code that generates plots for a better modularity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(don't have to make this change if integrating the code is more urgent)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is separated in the new commits.
class lickMetrics: | ||
"""calculate lick quantiles""" | ||
|
||
def __init__(self, nwb): | ||
"""Input: nwb of behavior data""" | ||
self.sessionID = nwb.session_id | ||
self.tblTrials = nwb.trials.to_dataframe() | ||
self.leftLicks = nwb.acquisition["left_lick_time"].timestamps[:] | ||
self.rightLicks = nwb.acquisition["right_lick_time"].timestamps[:] | ||
self.allLicks = np.sort( | ||
np.concatenate((self.rightLicks, self.leftLicks)) | ||
) | ||
self.lickLat = ( | ||
self.tblTrials["reward_outcome_time"] | ||
- self.tblTrials["goCue_start_time"] | ||
) | ||
self.lickLatR = self.lickLat[self.tblTrials["animal_response"] == 1] | ||
self.lickLatL = self.lickLat[self.tblTrials["animal_response"] == 0] | ||
self.thresh = [0.05, 0.5, 1.0] | ||
self.kernel = norm.pdf(np.arange(-2, 2.1, 0.5)) | ||
self.binWidth = 0.2 | ||
self.binSteps = np.arange(0, 1.5, 0.02) | ||
self.lickMet = None | ||
self.winGo = [self.thresh[0], 1.0] | ||
self.winBl = [-1, 0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the same reason, in general, we discourage the use of classes in the shared repo. For example, this class can be refactored as:
- a function that extracts data from nwb (could be the same as the one mentioned at Lines 29~37) and returns
tblTrials
,leftLicks
andrightLicks
etc - a function
calMetrics
that works on the extracted data and returns the dictionarylickMet
- a plot function that receives
lickMet
and does the plotting.
But if you think it is unlikely that someone will reuse the functions independently of the class, we can leave them as they are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original version was indeed three separate functions, they were later combined because they share a lot of inputs. It's easy to split them!
|
||
lickSum = lickMetrics(nwb) | ||
lickSum.calMetrics() | ||
fig = lickSum.plot() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be
fig = lickSum.plot() | |
fig, _ = lickSum.plot() |
as lickSum.plot() returns two variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I also corrected other similar cases.
Have you pushed your latest changes for review? |
I added some new figures and also merged main branch in. It is ready for review now! |
Added two lick-focused analysis:
plotLickAnalysis
: plot lick rasters and lick rates aligned to the time of go cueslickMetrics
: calculate lick metrics and plot summary plots.Both of them take nwb as input and generate a figure and a string for sessionID.
Example plots can be found here:
plots
Example usage is here:
https://github.com/AllenNeuralDynamics/aind-dynamic-foraging-basic-analysis/blob/sueTest/src/aind_dynamic_foraging_basic_analysis/lickAnalysis.py