-
Notifications
You must be signed in to change notification settings - Fork 51
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
WIP mb/image2pulsetrain #54
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.
LGTM. Just had a few comments/questions, but nothing is crucial. Would be good to have tests, though.
const_amp=20, const_freq=20, rftype='gaussian', rfsize=None, | ||
tsample=0.005 / 1000, dur=0.5, pulsedur=0.5 / 1000., | ||
interphasedur=0.5 / 1000., pulsetype='cathodicfirst'): | ||
try: |
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.
Maybe only try to import skimage if a string is provided, or if the image is RGB? We could allow a simple gray-scale input, if people want to provide that directly.
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.
Agree, except we always need it for resizing the image accordingly... (see below)
# Convert image to grayscale | ||
img_large = rgb2gray(np.array(img)).astype(np.float32) | ||
|
||
# Maximize contrast |
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.
Wouldn't this lead to confusing results in an experiment in which different contrasts are used?
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.
Good point. Added an input argument max_contrast
so users can choose.
yhi = np.max(xyr[:, 1] + xyr[:, 2]) | ||
|
||
# Resize the image accordingly and flip up down | ||
img_resize = resize(img_large, (yhi - ylo, xhi - xlo)) |
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.
I guess we'll always need skimage for this...
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.
Yup, unfortunately...
for e in implant: | ||
rf = receptive_field(e, xg, yg, rftype, rfsize) | ||
magn.append(np.mean(rf * img_resize)) | ||
magn = np.array(magn) |
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.
We might be able to preallocate this one.
e_s += "'frequency'." | ||
raise ValueError(e_s) | ||
|
||
pt = Psycho2Pulsetrain(tsample, freq=freq, amp=amp, dur=dur, |
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.
Maybe there's a way to speed up/save memory by doing these all in the same time-series object? OTOH, might not be worth the hassle.
Work in progress: Add functionality to encode an image as a list of pulse trains.