4
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ Welcome to Codereview! This is a nice first question. Could you please tell us what Python version are you using ? \$\endgroup\$ – Grajdeanu Alex. Apr 25 '17 at 7:17
  • \$\begingroup\$ Python 3.5 (edited question to include this as well) \$\endgroup\$ – Simon Apr 25 '17 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\$ – MPath Apr 25 '17 at 9:35
2
\$\begingroup\$

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)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.