Image width and height setting no longer public exposed

Issue #1166 closed
Kit Choi
created an issue

In openpyxl 2.4, one can specify the size of an image being written to a worksheet by providing the size argument in Image.__init__.

In openpyxl 2.5, this argument is no longer available in Image.__init__

However, if I add the width and height attributes to the instance directly like this:

from openpyxl.drawing.image import Image
image = Image(stream)
image.width = 100
image.height = 100

Then it does get picked up by the anchor created in openpyxl.drawing.spreadsheet_drawing._check_anchor and it works.

Would it make sense to reinstroduce the size argument in Image.__init__?

Now I have code that add the width and height attributes directly on the instance, but I find it extremely uncomfortable that this is undocumented.

The change to removing size in Image.__init__ seems to me a bug because I can't otherwise see how the size can be set from the public interface.

Comments (9)

  1. CharlieC

    The change was made to provide a consistent API for any graphical elements. These are always positioned on a canvas using anchors for sizing. So, no, the parameters won't be coming back to the constructor. What you need to do is either create your own anchor and pass it in when you add the image to a worksheet, or alter the default anchor that is created automatically.

    Contributions to the documentation are always welcome.

  2. Kit Choi reporter

    Thank you Charlie.

    Is setting Image.width and Image.height the expected usage that can be documented?

    alter the default anchor that is created automatically.

    Do you mind explaining how the default anchor can be altered? Do you mean assigning Image.anchor directly?

  3. Kit Choi reporter

    either create your own anchor and pass it in when you add the image to a worksheet

    On creating my own anchor, the alternative to assigning Image.width and Image.height seem to involve repeating much of the code in openpyxl.drawing.spreadsheet_drawing._check_anchor, which involves more obviously private attributes. E.g. The OneCellAnchor needed a _from parameter. The naming with a preceding underscore indicates that this is a private argument that users of the library should not mess with. Is there a way to create an anchor for this use case from the more obviously public interface?

  4. CharlieC

    _from is in this case not private, just aliased because it's a Python keyword so can't be used as an attribute. Difficult to do this but for readability I prefer prefixing as opposed to suffixing (which SQLAlchemy uses).

    If you just want to resize the image, then do this with PIL before passing it to openpyxl. Anchors allow other kinds of manipulation and transformation (the sort of stuff that you can do in the Excel GUI). Unfortunately anchors require you to familiarise yourself with DrawingML but that's just how it works. The previous approach ellided this and made things like read support almost impossible.

    ws.add_image(img, anchor_or_coord)

    _check_anchor is essentially a factory function for creating a particular kind of anchor. So, this could be factored into a function that could be used both internally and externally. Otherwise, just cargo cult it to get started.

  5. Kit Choi reporter

    I see. Thank you. I got used to assuming anything preceded with an underscore is not to be messed with or I am asking for breakage :)

    On resizing the image with PIL: this leads to a resized image being written to the excel package. For my use case, the user wants to be able to enlarge an image manually (on MS Excel) and be able to see fine details. Hence resizing the actual binary image file is not an option.

    I don't mind going the more advanced route of creating an anchor. I do care about using something that is obviously public such that backward compatibility / deprecation process can be assumed.

    Just to clarify, it sounds as though I should not be setting Image.width and Image.height if I care about backward compatibility, but instead I should be implementing my own anchor following _check_anchor (which uses OneCellAnchor and pixel_from_EMU. Is that correct?

    For example, if I want to push documentation changes, should I document the use of Image.width and Image.height at all?

    Thanks again for your help!

  6. CharlieC

    Well, the convention is, of course, that a preceding underscore is private. But that was the best solution I could come up with to handle the automatic conversion to and from XML.

    The use case you're describing is exactly why anchors need to be first class citizens, a bit more work for the user, but not really possible with the older API.

    The Image class doesn't really need much documenting. Better to concentrate on using anchors (oneCell and twoCell). Yeah, pixel_to_EMU handles the conversion, though in theory other units are possible. I'm afraid I can't really say much about offsets, transformations or scaling. You're likely to have to work this out through reverse engineering.

    I'd prefer future discussion either to be on a PR or on the mailing list.

  7. Kit Choi reporter

    Thank you very much for the clarification. I think I understand the expected usage now. I am not sure whether you would suggest closing this issue and/or reopening a separate one for the fact that Image.width and Image.height are supported and yet there is no public interface to get to it?

  8. CharlieC

    This is more a discussion than an issue. The Image class is possibly not even really necessary except that it knows how to get relevant metadata.

    No new issue required, largely because you're looking backwards at code that was never really supported. You are welcome to submit a PR with either improved documentation or some kind of anchor factory.

  9. Log in to comment