Cannot add an image to a file with VBA

Issue #464 resolved
created an issue

Adding an image to a file with a macro leads to duplicate images and relations.

Comments (17)

  1. CharlieC reporter

    Looks like the conflict is provoked by relying on ARC_VBA greedily preserving possibly related items (anything in media or drawing paths).

  2. CharlieC reporter

    To resolve this is probably going to involve much more extensive support for reading files (images might be possible after 2.4). A pull request would expedite this.

  3. John Bovey

    I have downloaded the files and started to look into this. Clearly, adding images is broken in keep-vba mode, especially if the spreadsheet already contains an image. I am puzzled though, why the new image is being added @Jean-Baptiste Quenot. It looks identical to the one that is already in the template.

  4. CharlieC reporter

    @jbovey The problem is that the image isn't being preserved as fas as openpyxl is concerned (2.3 is in much better shape for doing this in the future but we don't at the moment) but keep_vba keeps all media files and thus causes at least a name conflict. keep_vba probably has to be a little pickier about what binary files it keeps.

  5. John Bovey

    There seems to be 3 separate issues:

    1. When not in keep_vba mode images are not preserved.

    2. When in keep_vba mode images are preserved in the xml file but they do not display in the worksheet.

    3. In keep-vba mode, any images added in a script conflict with the existing (but invisible) images and cause an invalid workbook file.

    If I can fix 2 then the OP should not need to add images and so should be able to get on with what he is trying to do. I'll have a look at 3 as well but I would not want to conflict with any plans you might have to fix 1.

  6. CharlieC reporter

    2 + 3 are related due to handling the images independently of the rest of the library. The VBA code needs to concentrate on managing only the assets directly related to VBA. Some of this stuff isn't easy.

  7. CharlieC reporter

    I didn't have to fix any code as a result of the changes but support for some of these discrete elements has always been flaky when they occur in the same file. This isn't helped by the frankly schizophrenic nature of the data (workbook global but worksheet local) E.g prior to 2.3 you couldn't have a hyperlink and an image or a chart in the same file. "Smuggling" stuff as we do to support VB was always likely to cause problems as a result. It might be possible simply to add the images to the workbook's list of images so that the counting is correct. This would be essentially the missing step to providing read-write support for images. But my eyes start bleeding every time I'm faced the dependency graph!

    Thanks very much for continuing to look at this.

  8. John Bovey

    I started by working with 2.3 and could not understand where all my changes from March had gone. Hence, the remark above about VBA being badly broken. Then I realised that they were still in 2.2. Presumably they will be merged into 2.3 at some point? I am now working with 2.2. I am slimming down the list of files that are preserved under keep_vba to just the ones that are needed. Some that I included originally were there because I thought they were needed for controls, but that does not seem to be the case. There is still the problem that vba controls are incompatible with comments. It would be better to raise an exception rather than generate a broken excel file as happens at the moment. Do you have any opinion about the best exception to use?

  9. CharlieC reporter

    2.2 is already merged into 2.3, this is always the case that new includes any changes to the current release.

    I had to make more substantial changes based on the chart rewrite (charts need drawings, etc.) but this makes stuff slightly saner. To my mind at least!

    I tend to prefer standard Python exceptions wherever possible - specific ones only make sense when they are intended to be part of flow control and caught be corresponding exception handlers. Otherwise it might make sense to apply the Highlanger principle - you can only have either VB or images but not both - and emit a warning. Otherwise perhaps a ProgrammingError or ArchiveError would seem reasonable.

  10. John Bovey

    I am not sure what went wrong but I have deleted my repository and taken a new branch which seems ok.

    I will be away tomorrow but I should have a pull request by the end of the week.

  11. CharlieC reporter

    The main change between 2.2 and 2.3 is using the Relation class to manage serialisation as opposed to using the XML libraries directly. The removes the need for individual counters for the relation ids.

    More and more code is following this approach with the aim of simply modelling objects in Python and relying on the library to handle transformation to and from XML.

  12. Log in to comment