Excel To CSV Converter Ch 9 ATBS I am learning more about python using the book "Automate The Boring Stuff". The problem I completed was "Excel To CSV Converter". I want to kick bad habits to the curb, and make sure I'm learning the best python practices to further my development. I would really appreciate feedback on code efficiency, and code neatness. Is my code PEP-8 compliant? Does it implement all the best practices possible? Thank you in advance, all feedback is appreciated and considered!

import openpyxl, csv, os, re, shutil

path = os.getcwd()
new_path = path + '\\' + 'csv_files'

for excel in os.listdir('.'):

    if not excel.endswith('xlsx'):
        continue

    workbook = openpyxl.load_workbook(excel)

    for sheets in workbook.sheetnames:
        wb_name = re.sub('.xlsx', '', excel)
        csv_name = wb_name + '_' + sheets + '.csv'
        csv_file = open(csv_name, 'w', newline='')
        csv_writer = csv.writer(csv_file)
        sheet = workbook.active

        for row_number in range(1, sheet.max_row + 1):
            row_info = []

            for col_number in range(1, sheet.max_column + 1):
                data = sheet.cell(row=row_number, column=col_number).value
                row_info.append(data)

            csv_writer.writerow(row_info)

        csv_file.close()
        shutil.move(os.path.join(path, csv_name), os.path.join(new_path, csv_name))
There are a few things you could improve. First, the most important if you want to write Python code that scales to more than a script that does just one thing, you should write functions (or classes). They have the nice feature that you can give them a name, which tells you roughly what they do, and you can give them a docstring which can describe what they do in even more detail.



Bug:

While you do iterate over the sheet names, you do not actually change sheets. workbook.active does not automatically switch to another name. Instead, directly iterate over the workbook.



Otherwise here are a few suggestions:


Instead of using os.listdir, you can use pathlib.Path, which has a glob method, to get all excel files in the current directory:

from pathlib import Path

excel_files = Path(".").glob("*.xlsx")


This way you also don't have to deal with different path delimiters in Unix and Windows and manually getting the current directory.

Python can also work with relative paths, so no need to get the current directory at all.
You should use the with keyword to ensure that the files you open are closed, even in the event of an exception.
openpyxl sheets have the attributes rows and columns, which lets you easily iterate over them. You should also learn about list comprehensions. In addition, a csv.writer has the writerows method that can take an iterable of rows to write.
Using regex just to replace a single string with nothing is overkill. You could just use str.replace. But here building the string constructively is probably easier. Learn about f-strings in Python 3.6+.
Instead of moving the file afterwards with shutils, directly save it to the right file.
Use a if __ name__ == "__main__": guard to allow importing from this script from another script.


With these changes your code could become:

import csv
import openpyxl
from pathlib import Path

def convert_all_sheets_to_csv(file_name, target_dir="."):
    """Convert all sheets to csv files saved in the folder `target_dir`."""
    workbook = openpyxl.load_workbook(file_name)
    out_file_name_template = Path(target_dir) / Path(file_name).stem
    for sheet in workbook:
        out_file_name = f"{out_file_name_template}_{sheet.title}.csv"
        convert_sheet_to_csv(sheet, out_file_name)

def convert_sheet_to_csv(sheet, file_name):
    """Convert the content of an excel sheet to a csv file."""
    with open(file_name, "w") as f:
        writer = csv.writer(f)
        writer.writerows([cell.value for cell in row] for row in sheet.rows)

if __name__ == "__main__":
    for excel_file in Path(".").glob("*.xlsx"):
        convert_all_sheets_to_csv(excel_file, target_dir="csv_files")