patternpythonMinor
XLSX writer implementation
Viewed 0 times
writerxlsximplementation
Problem
We have multiple scripts which write into XLSX with the same format, into a different format or slide variation. I am trying to write an API which saves time for other programs, but I need input on restructuring the file.
```
import openpyxl
from xlsxwriter.workbook import Workbook
class xlsx_extend(object):
"""This class contain api of xlsx functionality"""
def __init__(self, name):
"""create xls file"""
self.xlsx_name = Workbook(name)
self.create_format('blue', 12)
def create_format(self, color, size):
"""write header"""
self.xlsx_format = self.xlsx_name.add_format()
self.xlsx_format.set_font_size(size)
self.xlsx_format.set_bold()
self.xlsx_format.set_color(color)
self.xlsx_format.set_border()
self.xlsx_format.set_center_across()
self.xlsx_format.set_align('center')
self.xlsx_format.set_rotation(90)
def add_sheet(self, ws_name):
"""Adding worksheet name"""
return self.xlsx_name.add_worksheet(ws_name)
def rotate_text_format(self, angle):
"""rotate the text data"""
self.xlsx_format.set_rotation(angle)
def get_xlsx_name(self):
"""Returing xlsx name for derive class"""
return self.xlsx_name
def close_xlsx(self):
"""close xlsx File"""
self.xlsx_name.close()
class xlsx_sheet:
"""This is xlsx worksheet class"""
def __init__(self, excel_sheet, ws_name):
"""Initialze Base function and Create Worksheet"""
self.excel_sheet = excel_sheet
self.ws_name_worksheet = excel_sheet.add_sheet(ws_name)
self.ws_name_worksheet.set_column(0, 0, 40)
def write(self, row, col, data, format=None):
"""write Add in xlsx File"""
self.ws_name_worksheet.write(row, col, data)
def write_column(self, row, col, data, format=None):
"""write data in column than need pass data as list"""
self.ws_name_worksheet.write_column(row, col
```
import openpyxl
from xlsxwriter.workbook import Workbook
class xlsx_extend(object):
"""This class contain api of xlsx functionality"""
def __init__(self, name):
"""create xls file"""
self.xlsx_name = Workbook(name)
self.create_format('blue', 12)
def create_format(self, color, size):
"""write header"""
self.xlsx_format = self.xlsx_name.add_format()
self.xlsx_format.set_font_size(size)
self.xlsx_format.set_bold()
self.xlsx_format.set_color(color)
self.xlsx_format.set_border()
self.xlsx_format.set_center_across()
self.xlsx_format.set_align('center')
self.xlsx_format.set_rotation(90)
def add_sheet(self, ws_name):
"""Adding worksheet name"""
return self.xlsx_name.add_worksheet(ws_name)
def rotate_text_format(self, angle):
"""rotate the text data"""
self.xlsx_format.set_rotation(angle)
def get_xlsx_name(self):
"""Returing xlsx name for derive class"""
return self.xlsx_name
def close_xlsx(self):
"""close xlsx File"""
self.xlsx_name.close()
class xlsx_sheet:
"""This is xlsx worksheet class"""
def __init__(self, excel_sheet, ws_name):
"""Initialze Base function and Create Worksheet"""
self.excel_sheet = excel_sheet
self.ws_name_worksheet = excel_sheet.add_sheet(ws_name)
self.ws_name_worksheet.set_column(0, 0, 40)
def write(self, row, col, data, format=None):
"""write Add in xlsx File"""
self.ws_name_worksheet.write(row, col, data)
def write_column(self, row, col, data, format=None):
"""write data in column than need pass data as list"""
self.ws_name_worksheet.write_column(row, col
Solution
Naming Conventions
According to the official Python style guide, class names should be in
In your case, I would capitalize the entire
There also seems to be some redundancy in some of your names:
This next point may be speaking on an intentional design measure, but I would take a look at what properties are supposed to be 'public' and which are meant to be 'private'. Even though Python doens't truly have private variables, it is still convention to use a single underscore ('_') at the start of any intended private variables.
Some of your names are misleading:
Structure
I am not a big fan of declaring instance variables outside of the
Then just put this line in the
Also, you have an accessor method (which is nice) for
The same goes for your
Essentially you are circumventing all the work you did wrapping the
Since your
According to the official Python style guide, class names should be in
PascalCase:No:
class my_class():
Yes:
class MyClass():In your case, I would capitalize the entire
xlsx because it is an acroynm:class XSLXExtend():There also seems to be some redundancy in some of your names:
# worksheet_name_worksheet?
self.ws_name_worksheetThis next point may be speaking on an intentional design measure, but I would take a look at what properties are supposed to be 'public' and which are meant to be 'private'. Even though Python doens't truly have private variables, it is still convention to use a single underscore ('_') at the start of any intended private variables.
self._i_should_be_private = 'No touching!'Some of your names are misleading:
# Wait. This stores the actual Workbook not its name?
self.xlsx_name = Workbook(name)Structure
I am not a big fan of declaring instance variables outside of the
__init__ method. In lieu of this, I would refactor your create_format() method as such:def create_format(self, color, size):
format = self.xlsx_name.addFormat()
format.set_font_size(size)
format.set_bold()
format.set_color(color)
format.set_border()
format.set_center_across()
format.set_align('center')
format.set_rotation(90)
return formatThen just put this line in the
__init__ method: self.format = create_format('blue', 12).Also, you have an accessor method (which is nice) for
xlsx_name. However, in your code that actually uses your classes, you just directly access it:# Why even have `get_xlsx_name()` if you don't use it?
red_format = report_xlsx.xlsx_name.add_format(...)The same goes for your
create_format() method. You use it once in __init__ but don't when you actually use your class. You skip straight to the Workbook:red_format = report_xlsx.xlsx_name.add_format({'bg_color': '#FF0000'})Essentially you are circumventing all the work you did wrapping the
Workbook class in another class.Since your
xlsx_extend class is supposed to store data about and manipulate a Workbook why not store all of the formats there? Here is what I would do:class XLSXExtension(object):
def __init__(self, name):
self.xlsx_name = Workbook(name)
self.formats = {}
self.create_format('blue', 'blue')
def create_format(self, name, color, size=12):
format = self.xlsx_name.addFormat()
format.set_font_size(size)
format.set_bold()
format.set_color(color)
format.set_border()
format.set_center_across()
format.set_align('center')
format.set_rotation(90)
self.formats[name] = format
def __getattr__(self, key):
try:
return self.formats[key]
except KeyError:
print('Format "{}" not found.'.format(key))
report_xlsx = xlsxutils.xlsx_extend("report.xlsx")
report_xlsx.create_format('red', '#FF0000')
report_xlsx.create_format('green', '#99CC32', 24)
report_worksheet = xlsxutils.xlsx_sheet(report_xlsx, "Report")
report_worksheet.text_condition_format(1, 1, row, col, 'P', report_xlsx['green'])
report_worksheet.text_condition_format(1, 1, row, col, 'F', report_xlsx['red'])Code Snippets
No:
class my_class():
Yes:
class MyClass():class XSLXExtend():# worksheet_name_worksheet?
self.ws_name_worksheetself._i_should_be_private = 'No touching!'# Wait. This stores the actual Workbook not its name?
self.xlsx_name = Workbook(name)Context
StackExchange Code Review Q#39792, answer score: 3
Revisions (0)
No revisions yet.