implementing replace_picture() #6
Conversation
We iterate over all shapes find_shapes() return. Raises a FileNotFoundError if `filename`does not point to a file. The picture is created as a new shape The last step is to delete the old shape.
The code uses data/example01.pptx to test adding an image shape. The placeholder with the text #logo gets replaced by the image.
We iterate over all shapes find_shapes() return. Raises a FileNotFoundError if `filename`does not point to a file. The picture is created as a new shape The last step is to delete the old shape.
| img_file = open(filename, "rb") | ||
| old_shape: BaseShape | ||
| for old_shape in shapes_to_replace: | ||
| slide_shapes = old_shape._parent | ||
| slide_shapes.add_picture( | ||
| image_file=img_file, | ||
| left=old_shape.left, | ||
| top=old_shape.top, | ||
| width=old_shape.width, | ||
| height=old_shape.height | ||
| ) |
There was a problem hiding this comment.
Should this be a with open() …?
Or is that not required when only read access is performed?
| filename = pathlib.Path(filename) | ||
| if not filename.is_file(): | ||
| raise FileNotFoundError(f"The file does not exist: {filename}") | ||
| img_file = open(filename, "rb") |
There was a problem hiding this comment.
| img_file = open(filename, "rb") | |
| with open(filename, "rb") as img_file: |
| if not filename.is_file(): | ||
| raise FileNotFoundError(f"The file does not exist: {filename}") | ||
| img_file = open(filename, "rb") | ||
| old_shape: BaseShape |
There was a problem hiding this comment.
I would maybe type shapes_to_replace, instead of old_shape.
There was a problem hiding this comment.
I already use shapes_to_replace for the iterator that is returned by _find_shapes(). In my opinion the singular version shape_to_replace is to close to shapes_to_replace and therefor old_shape makes it easier to distinguish between the iterator and the single shape.
We use requirements.txt and dev_requirements.txt. Therefore, Pipfile and Pipfile.lock are not needed. Ignoring all PyCharm related files in .idea/.
We use requirements.txt and dev_requirements.txt. Therefore, Pipfile and Pipfile.lock are not needed. Ignoring all PyCharm related files in .idea/.
Running replace_by_image() a second time. verifying the second run does not create new shapes.
| # install all needed dependencies. | ||
| #Pipfile.lock | ||
| Pipfile | ||
| Pipfile.lock |
There was a problem hiding this comment.
What's the reason for not committing the dependency lock file? I would only not do this if there is a problem...
There was a problem hiding this comment.
As we use requirements.txt and dev_requirements.txt for listing dependencies a Pipfile would duplicate the requirements.
| """ | ||
| pass | ||
| shapes_to_replace = self._find_shapes(label) | ||
| if not shapes_to_replace: |
There was a problem hiding this comment.
This should be an Exception. Don't let errors pass silently!
There was a problem hiding this comment.
It is a good idea to handle the problem on a higher level. I will replace return by
raise ValueError(f"The label '{label}' can't be found in the template.")
| img_file = open(filename, "rb") | ||
| old_shape: BaseShape | ||
| for old_shape in shapes_to_replace: | ||
| slide_shapes = old_shape._parent |
There was a problem hiding this comment.
Not a big fan of using the internals of the pptx library. But if there is really no better way...
There was a problem hiding this comment.
It seems there is currently no other solution:
scanny/python-pptx#246
Could be mentionend in a code comment.
| ) | ||
| # Removing shapes is performed at the lxml level. | ||
| # The `element` attribute contains an instance of `lxml.etree._Element`. | ||
| slide_shapes.element.remove(old_shape.element) |
There was a problem hiding this comment.
Yikes! We should really not deal with xml ourselves. We should open a RP in pptx to get this feature implemented. For the time being, we should either 1) isolate this in a private method marked deprecated to prevent excessive use, or 2) decorate the pptx presentation object.
Large images are always scaled. The aspect ratio is kept. The new kwarg `do_not_scale_up`can disable upscaling. Centering is always active.
Raising an error if `shapes_to_replace` is empty. Wrapping `open()`into a `with` statement.
5 participants
We iterate over all shapes
find_shapes()returns.Raises a
FileNotFoundErroriffilenamedoes not point to a file.The picture is created as a new shape.
The last step is to delete the old shape.