2
\$\begingroup\$

I'm looking to understand if my code has an obvious blockage or performance pain point that will cause it to operate slower or use more memory than it should.

The current Excelfile i am processing with this script has 5 sheets. the first sheet is the largest containing 6000 rows. I've never done this before so am unsure how long this should really take.

import pandas as pd
import glob
import os


"""
Retrieves csv files from folders and appends them to an existing Excel sheet. 

Reference

https://www.geeksforgeeks.org/how-to-append-data-in-excel-using-python/
https://www.geeksforgeeks.org/how-to-merge-multiple-csv-files-into-a-single-pandas-dataframe/
https://stackoverflow.com/questions/26521266/using-pandas-to-pd-read-excel-for-multiple-worksheets-of-the-same-workbook

"""

dirs = []
searchFiles = []
files = []

mainFile = os.path.join(os.getcwd(), "Phone Performance.xlsx")

xls = pd.ExcelFile("Phone Performance.xlsx")
sheetNames = xls.sheet_names

dirfiles = os.listdir(os.getcwd())

# creating directories to search
for file in dirfiles:
    if os.path.isdir(file) and not file.startswith("."):
        dirs.append(file + "\*.csv")


# yield the files to be processed
def getPath(directories):
    for item in directories:
        path = os.path.join(os.getcwd(), item)
        files.append(glob.glob(path))

    yield files


# use concat to process data files if there is single or multiple in a directory
def getData(directories):
    for file in getPath(dirs):
        for item in file:
            data = pd.concat(
                filter(lambda x: not x.empty, map(pd.read_csv, item)), ignore_index=True
            )

            yield data


# append new data to existing data for each sheet in the Excelfile
def existingData(file, name):
    count = 0
    while count <= len(name):
        existingData = pd.read_excel(file, name[count])
        yield existingData._append(next(getData(dirs)), ignore_index=True)
        count += 1


data = next(existingData(xls, sheetNames))

for sheet in sheetNames:
    with pd.ExcelWriter(xls, mode="a", if_sheet_exists="overlay") as writer:
        data.to_excel(xls, sheet, index=False)

# removing used files
for dir in dirs:
    if os.path.isfile(file):
        os.remove(file)

Is there something I can improve to make my process more performant?

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

This submission is about performance, yet it does not contain a single timing measurement.

unsure how long this should really take.

We expect that processing e.g. six 1 MiB workbooks should take twice as long as processing three 1 MiB workbooks. We want linear scaling with data volume, not quadratic.

CWD

dirfiles = os.listdir(os.getcwd())

It suffices to use "." to refer to current working directory.

Similarly for mainFile. Which would be easier to work with if it were a Path. And which pep-8 asked you nicely to spell as main_file.

helper functions

Thank you for defining a few helpers.

def getPath(directories):
    ...
def getData(directories):
    ...
def existingData(file, name):

Feel free to add type annotations to explain what they do. For example we apparently have name: list[str], suggesting it is actually plural sheet_names.

Rather than a # comment before the signature, please get in the habit of putting a """docstring""" right after each signature. It isn't easy to write clear docstrings. I recommend you write them down and then have a hallway conversation with a colleague, to see if they can accurately describe the inputs and outputs without reading any additional source code. Iterate until clarity is improved.

You have a confusing mixture of top-level scripting statements together with function defs. Prefer to define functions first, and then put scripting verbs within the usual guard:
if __name__ == "__main__":

quadratic algorithm

I get the sense that this is re-doing a lot of work, for \$O(n^2)\$ cost:

        yield existingData._append(next(getData(...

I wouldn't be surprised if all those repeated calls to pd.read_excel(file, name[count]) are behaving similarly.

Profile to verify that's where the time is spent.

It appears to me that the pandas API may be convenient, but it's not efficient when we're working with a great many sheets. Prefer the openpyxl package. That way you get an open workbook as a handle into the file, and you can efficiently iterate through its sheets without re-reading earlier sheets you've already dealt with. It is the re-reading that leads to quadratic complexity and slow execution.

Oh, wait. It looks like you could be using pd.read_excel(xls, name[count]), with similar effect to how openpyxl keeps the giant workbook open without re-reading rows.

Make the edit, writing down before-and-after timing measurements to see whether it affects elapsed time for the task.

There's also this form in the SO answer you cited:

sheet_to_df_map = pd.read_excel(file_name, sheet_name=None)

So None says, "please read every sheet", and then you can iterate over the resulting dict of sheets.

\$\endgroup\$

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.