#267 Merged at 9b54a9e
Repository
pjoyce
Branch
image_read
Repository
openpyxl
Branch
2.5
Author
  1. Paul Joyce
Reviewers
Description

This PR adds support for reading images embedded in worksheet. It is modelled on the way charts are read from a worksheet.

Comments (4)

  1. CharlieC

    Looks pretty good, thanks. Having had to fix numerous non-chart related bugs in spreadsheet drawing I’d be hoping that this would be fairly straightforward. I guess that ideally the code for charts and images should be in a single loop so that the drawing is only loaded once but I can look at doing that myself.

  2. Paul Joyce author

    Oh dear. I originally made the changes based on a different tip, and applied them back to 2.5 to submit a PR. The tip I used had an ExcelReader class in it, which is where the self came from. I thought I’d run the tests (py.test) on my final version, but I must not have. Apologies.

    Please run tox over the project before submitting future pull requests.

    I wasn’t really sure about the command line for this. Your documentation for submitting code https://openpyxl.readthedocs.io/en/stable/development.html says “Please use tox to test code”, but I’ve not used tox before so I wasn’t sure what to do for that. Is there a particular command line that should be run? Is just “tox” correct? In the Documentation section following, it gives an example “tox -e doc”, but this is for doco.

    Thanks, and thank you for a really useful open source project. We are using it to read in specifications authored as xlsx (which include images 🙂)

    1. CharlieC

      In general just running tox will be sufficient to run through all the combinations but you only need to do this once pytest is running fine. I don’t want to have to document tox…

      We have a different version strategy in openpyxl than many projects: tip is always the last stable release. This means that anyone who does a checkout will have a working version and no surprises. 2.6 is the current development version and has the ExcelReader class as part of some refactoring to make a fairly monolithic part a bit more testable.

      Documentation, especially for developers, is difficult. Suggestions, especially in the form of PRs, are always welcome.

      It’s great to see that you were able to get some reading done so easily! It’s very rewarding for me to see the lots of work done in 2.3 and 2.4 paying off in terms of extensibility. Unfortunately the files with images I have aren’t working yet, but I think it won’t be too hard to get them working. I was able to make the changes easily enough and also merge things back into 2.6. So thanks again and glad you’re finding the library useful.