spreadsheet with single value generates exception with worksheet.rows

Issue #599 resolved
Jon Joseph
created an issue

spreadsheet with single value generates exception with worksheet.rows

Comments (17)

  1. Jon Joseph reporter

    Can you get openpyxl version 2.3.3 to read the data in the attached file? If so then it's probably my bug. If I add a second value at B2 I can then read the file.

  2. Jon Joseph reporter

    Hi Charlie. It's being removed somewhere. It is simply an excel created file with one sheet and one string value in a1. The file is saved and I exit excel. The tuple returned when I ask for worksheet.rows is ((),) -->empty. If I put an additional value into b1 it all works.

  3. Charlie Clark

    If you're trying to add attachments by e-mail I assume they'll be stripped. But I think I can recreate the file. I don't see any exceptions but I think it isn't going to be tricky to fix.

    It will work in read-only mode because that doesn't need to have a floor for creating cells.

  4. Nicholas Pressey

    I can reproduce this issue with openpyxl 2.3.3:

    >>> from openpyxl import Workbook
    >>> wb = Workbook()
    >>> ws = Workbook.active
    >>> ws['A1'] = 'First'
    >>> ws.rows
    >>> ws['B1'] = 'Second'
    >>> ws.rows
    ((<Cell Sheet.A1>, <Cell Sheet.B1>),)


    The problem appears to be in line 712 of worksheet.py:

    if self.min_row == self.max_row == self.min_column == self.max_column:
        return ((),)

    Since min_column and max_column don't change if a value is added to A1, it's returning ((),) instead of the correct value. Maybe changing it to something like this would work?

    if self.min_row == self.max_row == self.min_column == self.max_column and self['A1'].value is None:
        return ((),)
  5. Charlie Clark

    I'm slightly worried about all the interest in such a minor issue. As you've all seen there's a problem in that worksheets are "bootstrapped" so that the metrics for an empty worksheet are the same as for one containing a single cell in the first column of the first row.

    The best solution may be a proper handling of an empty worksheet.

    I wonder what the use case is for looping over the rows of a worksheet with just one cell in them. But here tuple(ws.iter_rows()) should work fine.

  6. Peter Pul

    @Charlie Clark As my pull request shows, my interest in columns has a different reason, as my pull request shows. My PR has a different solution for this problem. Don't know if this is the location to discuss it, but this way @Nicholas Pressey is up to date.

    dimension = self.calculate_dimension().upper()
    if self['A1'].value is None and dimension == "A1:A1":
        return ((),)

    Edit: Just read my email and was notified a problem with calculate_dimension

  7. Charlie Clark

    One thing to note here is that some code is shared across different implementations which makes a solution like using if not ws._cells… (always True in read-only modus) a trickier. But essentially there needs to be a check for whether a worksheet is empty or not.

    That aside, I think that ws.rows and ws.columns should be generators in both implementations.

  8. Log in to comment