I wanted to make use of the speedtest-cli while also incorporating pandas into the project. My initial goal, which I am posting here, was to take the values of the speed test and incorporate them into a CSV. I've been learning about Pandas and figured it would be a good way to accomplish the project.

import speedtest
import pandas as pd
import os

# Initial Speedtest variables
s = speedtest.Speedtest()
servers = []

# File path variables
localpath = ''
filepath = ''
filename = ''

# Functions #

def speedtest():
    """Makes use of the speed-test-cli python wrapper
    to get speed test data and returns in a dictionary"""
    results_dict = s.results.dict()
    return results_dict

def convert_mbps(arg):
    arg = arg / 1000000
    return arg

def csv_upload(localpath, filepath, filename):
    """Attempts to append a csv file and, if none
    exists, creates one in a specified path"""
        df2 = pd.read_csv(localpath)
        df2 = df2.append(df)
        df2.to_csv(os.path.join(filepath, filename), index=False)
    except OSError:
        df.to_csv(os.path.join(filepath, filename), index=False)

# Speedtest and convert to Data Frame
result = speedtest()
df = pd.io.json.json_normalize(result)
df2 = None

# Uploads CSV
csv_upload(localpath, filepath, filename)

I've tried as best as I could to document throughout my code, as well as to not statically specify parameters within functions; something which I've been bad for in the past. I've also tried to incorporate in some exception handling, though this is something that I believe needs more work in my code.

I've tested the code within a cron, it does exactly what it seems to be functioning as I would like it to. Future developments will surround analyzing data collected from the CSV; trying to determine trends in bandwidth or latency as a function of time of day. Any feedback would be most appreciated.


1 Answer 1

  1. Don't use globals. E.g., you refer to df and df2 in csv_upload, why not make them parameters?

  2. Use the standard if __name__ == '__main__': guard around the script's main body.

  3. It's common to just accept a flat file path, and expect the calling code to perform any os.path.join in the creation of said path.

  4. "upload" sounds like network activity, but you're saving the data to disk. This should probably follow a more typical naming scheme of to_csv or save or dump or something of that sort.

  5. This seems like a prime candidate for being a class.

I'd suggest something like the following:

import speedtest
import pandas as pd
import os

class MySpeedtest:
    def __init__(servers, initial_data_path=None):
        # Initial Speedtest variables
        self.s = speedtest.Speedtest()
        self.servers = servers

        if initial_data_path is not None:
            self.df = pd.read_csv(initial_data_path)
            self.df = pd.DataFrame()

    def run_test(self):
        """Makes use of the speed-test-cli python wrapper
        to get speed test data and returns in a dictionary"""
        results_dict = self.s.results.dict()

    def convert_mbps(arg):
        return arg / 1000000
        # Actually, this should probably be "arg / (2**20)"
        # 1 MB = 2**20 B

    def to_csv(self, path):
        """Attempts to append a csv file and, if none
        exists, creates one in a specified path"""
        self.df.to_csv(path, index=False)

if __name__ == '__main__':
    # TODO: populate a list of servers, define localpath/filepath/filename
    tester = MySpeedtest(servers, localpath)
    tester.to_csv(os.path.join(filepath, filename))
  • \$\begingroup\$ I'll be the first to admit that classes are a weakness in how I code. I have implemented some trivial examples, but don't often come to the conclusion to use them within my own code. Considering that the initial script works fine, I'm assuming that a class would be useful within a project of a larger scope. Would you mind to give me a little bit of information on how incorporating the code into a class increases its usability? \$\endgroup\$ May 16, 2018 at 14:04
  • 1
    \$\begingroup\$ It compartmentalizes your speedtest logic, including related state, into a single entity. Since the functions rely on sharing state back and forth, it seems reasonable to code them in such a way that their state is stored in a traditional fashion (as class instances). This also increases usability and testability by creating an entity that can be instantiated and exhaustively tested; when the code relied on global variables, it would've been difficult to rigorously test when embedded in a larger application (which could've changed the global variables between calls, for example) \$\endgroup\$
    – scnerd
    May 16, 2018 at 17:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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