0
\$\begingroup\$

Context: This is a program that translates spreadsheets to English from Dutch using another custom library (transl).

Question:

Would like to hear criticism or advice on how to make the code better - not noob-like (Though I am one). Also, want to know what I do good.

Thank you.

Backstory: I Never got feedback on my code because I do it to facilitate my work. (I am not a programmer, but a business student)

from openpyxl import load_workbook
import transl
import logging

level = logging.INFO
fmt = '[%(levelname)s] %(asctime)s - %(message)s'
logging.basicConfig(level=level, format=fmt)


class Spreadsheet:

    def __init__(self, filename_):
        self.workbook = None
        self.filename = filename_

    @staticmethod
    def translate_cell(text):
        try:
            float(text)
        except ValueError:
            if text[0] != "=":
                return transl.translate(text)
            else:
                return text
        except TypeError:
            return ''
        else:
            return round(float(text), 2)

    def transl_sheet(self, sheet):
        for col_n, column in enumerate(sheet.iter_cols(values_only=True), start=1):
            for row_n, cell in enumerate(column, start=1):
                try:
                    sheet.cell(row=row_n, column=col_n).value = self.translate_cell(cell)
                except AttributeError:
                    pass
        logging.info(f'Worksheet "{sheet.title}" - Translation complete. Moving on...')
        return sheet

    def translate(self):
        logging.info(f'Loading the workbook.')
        try:
            self.workbook = load_workbook(filename=self.filename)
        except Exception as e:
            logging.error(f"Error occurred. Couldn't load the workbook. Possibly wrong filename.\n{e}")
        else:
            logging.info(f'Successfully loaded the workbook.')
            return [self.transl_sheet(sheet) for sheet in self.workbook]

    def save(self):
        name = f'Translated - {self.filename}'
        logging.info(f'Saving the workbook into "{name}"')
        self.workbook.save(filename=name)
        logging.info('Done.')


if __name__ == "__main__":
    spreadsheet = Spreadsheet("UvA-Budget_2023-2026.xlsx")
    spreadsheet.translate()
    spreadsheet.save()
```
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

For the most part this is good stuff! I only have a few pointers to give:

  1. Strings: Only use an f-string where necessary. Bits like logging.info(f'Loading the workbook.') should just be replaced with normal strings if you don't plan on interpolating any variables.
  2. Comments: Adding docstrings to functions will make the code much more useful to anyone who may want to touch your script in the future, because they'll be able to take one glance at things and understand exactly what each code segment is responsible for.
  3. Extensibility: Instead of hard-coding your file name into your code, you could use argparse like the following to make it a CLI that you can use on the fly:
import argparse

...

parser = argparse.ArgumentParser()
parser.add_argument("-n", "--name", type=str, required=True, help="The name of the file")
args = parser.parse_args()

...

if __name__ == "__main__":
    spreadsheet = Spreadsheet(args.name)
    spreadsheet.translate()
    spreadsheet.save()

This will allow for you to invoke the file like ./filename.py -n UvA-Budget_2023-2026.xlsx instead of having to go into a text editor to make the change.

That's all I have for you, happy coding!

\$\endgroup\$
2
  • 1
    \$\begingroup\$ "Only use an f-string where necessary." I'm curious, why? I've recently gotten into the habit of using f-strings everywhere. It's easier to modify and extend. It's also easier to stay consistent that way. If there's a downside, I may have to reconsider that approach. \$\endgroup\$
    – Mast
    Dec 7, 2022 at 11:09
  • \$\begingroup\$ IMHO using standard strings where you have no intention of interpolating is better for readability, else it looks like you intend to return to the string and add variables. I feel it's better to be explicit where you are and are not intending to use vars with strings. Performance-wise they should be identical. \$\endgroup\$ Dec 7, 2022 at 15:29

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.