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 (18)

  1. CharlieC

    Can you please provide some more information such as a sample file and the Python code you're using?

  2. 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.

  3. Jon Joseph reporter

    I am not using read only mode. I do an openpyxl.load_workbook(filename). I
    reattached the file.

  4. 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.

  5. CharlieC

    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.

  6. 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 ((),)
  7. Jon Joseph reporter

    Yes. I'm sorry. The exception happens because I try to do something with the empty tuple in the next line of my code.

  8. CharlieC

    Well, you'll need to be able to handle that in your code. I'll see what I can about the single cell problem.

  9. Peter Pul

    I would recommend the answer of Nicholas Pressey, altough I would start with the is None test, to avoid the expansive min/max properties.

  10. CharlieC

    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.

  11. Peter Pul

    @charlie_x 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 @TheLetterN 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

  12. CharlieC

    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.

  13. Log in to comment