-
Notifications
You must be signed in to change notification settings - Fork 117
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
add amplitude parameter to sonify.pitch_contour #266
add amplitude parameter to sonify.pitch_contour #266
Conversation
Thanks @rabitt ! I am traveling until Monday, so if someone else with push access wants to CR before then feel free. Looks like for now at least the test needs to be updated to fix Travis. |
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.
ha! awesome, I wrote a worse version of this myself the other day. just the one q
tests/test_sonify.py
Outdated
|
||
# Test with explicit amplitude | ||
x = mir_eval.sonify.pitch_contour(times, freqs, fs, length=fs * 7, | ||
amplitude=amps) |
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.
does this work? isn't the kwarg named amplitudes
(plural)?
3c11083
to
b3e470e
Compare
b3e470e
to
3498505
Compare
My guess is that this is due to a breaking Matplotlib release. I will create a separate issue. |
…ify-pitch-contour-amplitude
@ejhumphrey @craffel I think this is good to go? |
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.
Minor docstring change otherwise good to go.
else: | ||
# build an amplitude interpolator | ||
a_interp = interp1d( | ||
times * fs, amplitudes, kind=kind, |
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.
Can you change the docstring for kind
so that it reads
Interpolation mode for the frequency and amplitude values.
Thanks! |
Adds an optional
amplitude
parameter tosonify.pitch_contour
that applies an amplitude envelope to the pitch contour.