-
Notifications
You must be signed in to change notification settings - Fork 13
Extending replace_picture()
to scale images:
#9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
e995f82
948ab46
1a3aca6
badd26f
2fe3714
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,14 +33,49 @@ def replace_text(self, label: str, new_text: str) -> None: | |
for shape in shapes: | ||
shape.text = new_text | ||
|
||
def replace_picture(self, label: str, filename: _Pathlike) -> None: | ||
def replace_picture(self, label: str, filename: _Pathlike, *, scale_up: bool = True) -> None: | ||
"""Replaces rectangle placeholders on one or many slides. | ||
|
||
The aspect ratio of the image is not changed. | ||
To large images are always resized. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To -> Too |
||
The behaviour for small images is configurable. | ||
Centering is always active. | ||
|
||
Args: | ||
label (str): label of the placeholder (without curly braces) | ||
filename (path-like): path to an image file | ||
scale_up (bool): deactivates that the image is enlarged (default: True) | ||
""" | ||
pass | ||
slide_number, tag_name = self._parse_label(label) | ||
shapes_to_replace = self._find_shapes(slide_number, tag_name) | ||
if not shapes_to_replace: | ||
raise ValueError(f"The label '{label}' can't be found in the template.") | ||
with open(filename, "rb") as img_file: | ||
old_shape: BaseShape | ||
for old_shape in shapes_to_replace: | ||
slide_shapes = old_shape._parent | ||
img_shape = slide_shapes.add_picture( | ||
image_file=img_file, | ||
left=old_shape.left, | ||
top=old_shape.top, | ||
) | ||
# Scaling the image if `scale_up == True`: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the DocString, images too large for the old_shape are resized to fit into old_shape. This feature is still missing! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really think that (at least) scaling the image should be delegated to a private method to improve readability. |
||
if img_shape.height <= old_shape.height and img_shape.width <= old_shape.width and scale_up: | ||
old_aspect_ratio = old_shape.width / old_shape.height | ||
new_aspect_ratio = img_shape.width / img_shape.height | ||
if old_aspect_ratio >= new_aspect_ratio: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using an image in portrait mode, the width of the image is set to the width of old_shape, which results in an overflow in y direction. This logic seems broken? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Vice versa, an image in landscape mode overflows in x direction. Replacing |
||
img_shape.width = old_shape.width | ||
img_shape.height = int(img_shape.width / new_aspect_ratio) | ||
else: | ||
img_shape.height = old_shape.height | ||
img_shape.width = int(img_shape.height * new_aspect_ratio) | ||
# Centering the image at the extent of the placeholder: | ||
img_shape.top += int((old_shape.height - img_shape.height) / 2) | ||
img_shape.left += int((old_shape.width - img_shape.width) / 2) | ||
del slide_shapes[slide_shapes.index(old_shape)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For me running on Python 3.6.9 (installed via pyenv, test suite is OK), the deletion is not working:
|
||
# 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) | ||
|
||
def replace_table(self, label: str, data) -> None: | ||
"""Replaces rectangle placeholders on one or many slides. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor rewording please: ... on one or many slides with an image.