2
\$\begingroup\$

This code returns a list of widgets from spreadsheet ranges that are enabled for processing. The code works but it is poorly implemented, especially in terms of proper use of zip and layout.

The code needs to run on various ranges but was initially hard-coded for development. In openpyxl row/column processing is more efficient than coordinate ranges and coordinate range functionality is progressively being depreciated. (The coordinate code is only used for development and will be deleted.)

It's my first list comprehension and getting into zip so I'd also appreciate advice on how to lay out the long single lines of code.

from __future__ import print_function
from openpyxl import Workbook
from openpyxl.utils import get_column_letter
import openpyxl


def do_widgets(ws, col0, col1):
    """ Process each widget. """
    NO_WDGTS = 5
    ENABLE_START_ROW = 6
    ENABLE_END_ROW = ENABLE_START_ROW + NO_WDGTS - 1
    WDGT_START_ROW = 1
    WDGT_END_ROW = WDGT_START_ROW + NO_WDGTS - 1

    # Coordinate code - delete whole block before implementation
    col_letter0 = get_column_letter(col0)
    col_letter1 = get_column_letter(col1)
    wdgos_range = [(col_letter0 + str(ENABLE_START_ROW) + ":" +
                    col_letter0 + str(ENABLE_END_ROW))]
    wdgos_range.append(col_letter1 + str(WDGT_START_ROW) + ":" +
                       col_letter1 + str(WDGT_END_ROW))
    # Delete
    # wdgos = [cell[1][0].value for cell in zip(ws['B6:B10'], ws['C1:C5'])
    #                                          if cell[0][0].value]
    wdgos = [cell[1][0].value for cell in zip(ws[wdgos_range[0]], ws[wdgos_range[1]])
                                             if cell[0][0].value]

    # Row/column code
    wdgts = [cell[1] for cell in zip([cell.value for col
                     in ws.iter_cols(min_row=ENABLE_START_ROW, min_col=col0,
                                     max_row=ENABLE_END_ROW, max_col=col0)
                     for cell in col],
                     [cell.value for col
                     in ws.iter_cols(min_row=WDGT_START_ROW, min_col=col1,
                                     max_row=WDGT_END_ROW, max_col=col1)
                     for cell in col]) if cell[0]]

    # Use list of enabled widgets
    print("Enable range [0], Widget range [1]")
    print(wdgos_range)
    for wdgo in wdgos:
            print(wdgo)
    print()
    print(wdgts, '\n')


wb = Workbook()
ws = wb.active

rows = [
    ['Number', 'Batch 1', 'Batch 2'],
    [2, 40, 30],
    [3, 40, 25],
    [4, 50, 30],
    [5, 30, 10],
    [0, 0, 0],
    [0, 1, 0],
    [1, 1, 0],
    [0, 0, 1],
    [0, 1, 0],
]

for row in rows:
    ws.append(row)
# Worksheet, enable column, widget column
do_widgets(ws, 2, 3)

PEP8 Results:

Check results
=============

E501:25:80:line too long (85 > 79 characters)
E127:26:46:continuation line over-indented for visual indent
E128:30:22:continuation line under-indented for visual indent
E128:34:22:continuation line under-indented for visual indent
E128:36:22:continuation line under-indented for visual indent

Debug output:

Enable range [0], Widget range [1]
['B6:B10', 'C1:C5']
30
25
10

[30, 25, 10]
\$\endgroup\$
  • \$\begingroup\$ Am I going crazy, or does the cell[1] eliminate the need for a zip? Why don't you just write the second term in the zip? \$\endgroup\$ – Reinderien May 26 '18 at 12:21
  • \$\begingroup\$ @Reinderien - I think you misread the code. The output of the list comprehension is just a list of cell[1] values from the zip where cell[0] from the zip is non-zero. This is shown in debug output. \$\endgroup\$ – flywire May 27 '18 at 11:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.