I have three functions for building, applying, and plotting filters:
Build a filter:
def filter_build(frequency, sample_rate, filter_type, filter_order):
nyq = 0.5 * sample_rate
if filter_type == 'bandpass':
try:
normal_cutoff = (frequency[0] / nyq, frequency[1] / nyq)
except Exception as e:
print("Must provide tuple of frequency ranges for a bandpass filter")
raise
else:
try:
normal_cutoff = frequency / nyq
except Exception as e:
print("Must provide a single frequency value for this filter type")
raise
b, a = butter(filter_order, normal_cutoff, btype=filter_type, analog=False)
return b, a
Apply filter:
def filter(signal, frequency, sample_rate, filter_type, filter_order=2):
try:
b, a = filter_build(frequency, sample_rate, filter_type, filter_order)
except:
print("Error when creating filter")
return None
filtered = filtfilt(b, a, signal)
return filtered
Plotting the filter response:
def plot_filter_response(frequency, sample_rate, filter_type, filter_order=2):
try:
b, a = filter_build(frequency, sample_rate, filter_type, filter_order)
except:
print("Error when creating filter")
return None
w, h = freqz(b, a, worN=8000)
plt.subplot(2, 1, 1)
plt.plot(0.5*sample_rate*w/np.pi, np.abs(h), 'b')
plt.xlim(0, 0.5*sample_rate)
plt.title("Filter Frequency Response")
plt.xlabel('Frequency (Hz)')
plt.grid()
if filter_type == 'bandpass':
for i in range(len(frequency)):
plt.axvline(frequency[i], color='k')
plt.plot(frequency[i], 0.5*np.sqrt(2), 'ko')
else:
plt.axvline(frequency, color='k')
plt.plot(frequency, 0.5*np.sqrt(2), 'ko')
plt.show()
Everything appears to be working, but I dont think I'm handling exceptions correctly/efficiently.
For example, in filter()
and plot_filter_response()
I have the exact same try
block, which feels redundant
The purpose of the try
blocks (in all 3 functions) is to provide an easily understood error message if building a filter fails. The primary reason for this happening (that I can foresee) is if someone specifies a bandpass filter and only provides a single frequency cutoff, or if someone wants a lowpass filter but accidentally provides a tuple of 2 frequencies
Is there a way to remove the redundant try
blocks, so that if a filter is mis-specified, then filter_build()
will raise the error and stop any other lines of code from running?
A related issue is my inclusion of Return None
in the latter 2 functions. Is there a way to remove those? If I remove them, then currently if filter_build()
fails, the rest of the plotting code (or filtfilt
) still gets executed, which runs into more problems. Ideally I want the functions to just exit if filter_build()
doesn't return correctly
-
\$\begingroup\$ Welcome to Codereview! This is a nice first question. Could you please tell us what Python version are you using ? \$\endgroup\$Grajdeanu Alex– Grajdeanu Alex2017年04月25日 07:17:20 +00:00Commented Apr 25, 2017 at 7:17
-
\$\begingroup\$ Python 3.5 (edited question to include this as well) \$\endgroup\$Simon– Simon2017年04月25日 07:25:14 +00:00Commented Apr 25, 2017 at 7:25
-
\$\begingroup\$ When reusing code in multiple lines (as you are with the try blocks), it's usually a good idea to throw the redundant code into a function. Have you tried doing this? \$\endgroup\$user127168– user1271682017年04月25日 09:35:52 +00:00Commented Apr 25, 2017 at 9:35
1 Answer 1
Your code is buggy due to you guarding against Exception
, rather than a more specific exception.
This can lead to you saying a division by zero error is actually due to the user not passing in a number.
From this you have the following choices:
Change the errors to your own errors, whilst reducing the amount of errors you guard against in your try-excepts.
This would make
filter_build
look more like:def filter_build(frequency, sample_rate, filter_type, filter_order): nyq = 0.5 * sample_rate if filter_type == 'bandpass': try: normal_cutoff = (frequency[0] / nyq, frequency[1] / nyq) except TypeError: raise ValueError("Must provide tuple of frequency ranges for a bandpass filter") else: try: normal_cutoff = frequency / nyq except TypeError: raise ValueError("Must provide a single frequency value for this filter type") return butter(filter_order, normal_cutoff, btype=filter_type, analog=False)
This keeps the entire error stack, and makes debugging the code much easier, whilst keeping your error messages. But, you may want to change your approach, as you say you expect a tuple, rather than any indexable type. And so you could use the LBYL approach of:
def filter_build(frequency, sample_rate, filter_type, filter_order): nyq = 0.5 * sample_rate if filter_type == 'bandpass': if not isinstance(frequency, tuple): raise ValueError("Must provide tuple of frequency ranges for a bandpass filter") normal_cutoff = (frequency[0] / nyq, frequency[1] / nyq) else: if not isinstance(frequency, (float, int)): raise ValueError("Must provide a single frequency value for this filter type") normal_cutoff = frequency / nyq return butter(filter_order, normal_cutoff, btype=filter_type, analog=False)
However, you could always remove all the try-excepts, and just leave the default Python error messages. This can look like:
def filter_build(frequency, sample_rate, filter_type, filter_order): nyq = 0.5 * sample_rate if filter_type == 'bandpass': normal_cutoff = (frequency[0] / nyq, frequency[1] / nyq) else: normal_cutoff = frequency / nyq return butter(filter_order, normal_cutoff, btype=filter_type, analog=False)
I personally don't raise custom errors in most of my code, and so use the last option mostly. I think Python's errors are pretty good, and I'm kinda lazy. But all three options above are good.
You need to decide how you'll handle your errors. You want to either do no handling, and your program just exits with the error. Or you want to handle the error and display the actual error. To do the latter should instead look like:
import traceback def filter(signal, frequency, sample_rate, filter_type, filter_order=2): try: b, a = filter_build(frequency, sample_rate, filter_type, filter_order) except Exception: traceback.print_exc() return None return filtfilt(b, a, signal)
I'd highly recommend that you don't do the above. Maybe you have a usecase where it's good. But in almost all cases, it's better to remove the try-except, and for your program to exit.
If I were writing the above two functions I'd use the following:
def filter_build(frequency, sample_rate, filter_type, filter_order):
nyq = 0.5 * sample_rate
if filter_type == 'bandpass':
normal_cutoff = (frequency[0] / nyq, frequency[1] / nyq)
else:
normal_cutoff = frequency / nyq
return butter(filter_order, normal_cutoff, btype=filter_type, analog=False)
def filter(signal, frequency, sample_rate, filter_type, filter_order=2):
b, a = filter_build(frequency, sample_rate, filter_type, filter_order)
return filtfilt(b, a, signal)