Skip to content

Commit 29f3022

Browse files
Matthew HoroszowskiMatthew Horoszowski
authored andcommitted
feat(slides): add slide CRUD Phase 1 — remove, move, indexed add_slide
First slice of issue #11 (slide CRUD epic). Lands the in-deck CRUD operations users have been asking for since 2014; full duplicate(), cross-deck append_from(), and Sections support stay deferred to Phases 2–4 in the epic. New public API -------------- - `Slides.add_slide(slide_layout, index=None)` — `index=None` preserves existing append behavior (backwards compatible). Integer `index` inserts at that zero-based position; `index=len(self)` is a valid append, negatives raise `IndexError`. - `Slides.move(slide, new_index)` — reorder a slide in the presentation's slide sequence. Raises `ValueError` for foreign slides, `IndexError` for out-of-range `new_index`. - `Slides.remove(slide)` — drop a slide from the collection. The presentation→slide relationship is removed; the underlying part falls out on save. Image and other media parts shared with surviving slides are preserved (covered by integration test). - `Slide.delete()` — convenience alias for `prs.slides.remove(self)`. API shape mirrors the existing `SlideLayouts.remove(slide_layout)` shape rather than upstream PR scanny#1029's `remove_slide(slide_id)` — instance-keyed is more Pythonic and matches the rest of the library. Internal additions ------------------ - `CT_SlideIdList.insert_sldId_at(rId, idx)` — index-aware sldId insertion, with `idx == len` allowed. - `CT_SlideIdList.move_sldId_to(sldId, new_idx)` — reposition an existing sldId; no-op when the index already matches. - `CT_SlideIdList.remove_sldId(sldId)` — bounded remove with a parent-of check. Test coverage ------------- - 32 new unit tests in `tests/test_slide_crud.py` covering oxml helpers, the Slides CRUD methods, the Slide.delete alias, and four full-pipeline round-trip integration tests (add-at-index, remove, delete, move). Plus a shared-image-preservation round-trip and a `index=len(slides)` boundary test added per advisor review. - 5 new behave scenarios in `features/sld-crud.feature` exercising add-at-head, add-in-middle, move, remove, and Slide.delete. - New UAT script (untracked) at `uat_slide_crud.py` builds a deck exercising every new entry-point, saves, reopens, and prints a per-slide read-back so the maintainer can compare in PowerPoint / Keynote per CLAUDE.md §6. Tangential test-isolation fix ----------------------------- `tests/opc/test_package.py:DescribePartFactory` mutates `PartFactory.part_type_for[CT.PML_SLIDE]` to a class_mock and never reverts it. The mock leaked into any later test that loaded slides through `Presentation()` — caught here by the new round-trip integration tests. Restored via a request finalizer; isolated to that one test, no behavior change for the production code path. Verification ------------ ``` $ python3 -m pytest tests/ -q | tail -3 3086 passed in 3.76s $ ruff check src tests | tail -3 All checks passed! $ python3 -m behave features/ --no-color | tail -3 986 scenarios passed, 0 failed, 0 skipped 2952 steps passed, 0 failed, 0 skipped ``` Refs #11
1 parent 02049b3 commit 29f3022

6 files changed

Lines changed: 716 additions & 3 deletions

File tree

features/sld-crud.feature

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
Feature: Slide CRUD — remove, move, indexed add
2+
In order to programmatically assemble decks without lxml hacks
3+
As a developer using python-pptx
4+
I need to add slides at a chosen position, reorder them, and remove them
5+
6+
7+
Scenario: Slides.add_slide(slide_layout, index=0) inserts at the head
8+
Given a Slides object containing 3 slides
9+
When I call slides.add_slide(slide_layout, index=0)
10+
Then len(slides) is 4
11+
And the new slide is at index 0
12+
13+
14+
Scenario: Slides.add_slide(slide_layout, index=2) inserts in the middle
15+
Given a Slides object containing 3 slides
16+
When I call slides.add_slide(slide_layout, index=2)
17+
Then len(slides) is 4
18+
And the new slide is at index 2
19+
20+
21+
Scenario: Slides.move(slide, new_index) reorders slides
22+
Given a Slides object containing 3 slides
23+
When I call slides.move(slides[0], 2)
24+
Then len(slides) is 3
25+
And the slide order matches the original [1, 2, 0]
26+
27+
28+
Scenario: Slides.remove(slide) drops a slide and its rel
29+
Given a Slides object containing 3 slides
30+
When I call slides.remove(slides[1])
31+
Then len(slides) is 2
32+
And the surviving slide order matches the original [0, 2]
33+
34+
35+
Scenario: Slide.delete() removes the slide from its presentation
36+
Given a Slides object containing 3 slides
37+
When I call slides[1].delete()
38+
Then len(slides) is 2
39+
And the surviving slide order matches the original [0, 2]

features/steps/slides.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ def given_a_Slides_object_containing_3_slides(context):
2727
prs = Presentation(test_pptx("sld-slides"))
2828
context.prs = prs
2929
context.slides = prs.slides
30+
# ---capture original slide ids for CRUD ordering assertions---
31+
context.original_slide_ids = [s.slide_id for s in prs.slides]
3032

3133

3234
# when ====================================================
@@ -44,6 +46,33 @@ def when_I_call_slide_layouts_remove(context):
4446
slide_layouts.remove(slide_layouts[1])
4547

4648

49+
@when("I call slides.add_slide(slide_layout, index=0)")
50+
def when_I_call_slides_add_slide_index_0(context):
51+
layout = context.prs.slide_masters[0].slide_layouts[0]
52+
context.new_slide = context.slides.add_slide(layout, index=0)
53+
54+
55+
@when("I call slides.add_slide(slide_layout, index=2)")
56+
def when_I_call_slides_add_slide_index_2(context):
57+
layout = context.prs.slide_masters[0].slide_layouts[0]
58+
context.new_slide = context.slides.add_slide(layout, index=2)
59+
60+
61+
@when("I call slides.move(slides[0], 2)")
62+
def when_I_call_slides_move(context):
63+
context.slides.move(context.slides[0], 2)
64+
65+
66+
@when("I call slides.remove(slides[1])")
67+
def when_I_call_slides_remove(context):
68+
context.slides.remove(context.slides[1])
69+
70+
71+
@when("I call slides[1].delete()")
72+
def when_I_call_slide_delete(context):
73+
context.slides[1].delete()
74+
75+
4776
# then ====================================================
4877

4978

@@ -140,3 +169,26 @@ def then_slides_get_666_default_slides_2_is_slides_2(context):
140169
def then_slides_2_is_a_Slide_object(context):
141170
slides = context.slides
142171
assert type(slides[2]).__name__ == "Slide"
172+
173+
174+
@then("the new slide is at index {idx:d}")
175+
def then_the_new_slide_is_at_index(context, idx):
176+
assert context.slides[idx].slide_id == context.new_slide.slide_id, (
177+
"expected new slide at index %d, got slide_id mismatch" % idx
178+
)
179+
180+
181+
@then("the slide order matches the original [1, 2, 0]")
182+
def then_slide_order_matches_1_2_0(context):
183+
o = context.original_slide_ids
184+
expected = [o[1], o[2], o[0]]
185+
actual = [s.slide_id for s in context.slides]
186+
assert actual == expected, "expected %r, got %r" % (expected, actual)
187+
188+
189+
@then("the surviving slide order matches the original [0, 2]")
190+
def then_surviving_slide_order_matches_0_2(context):
191+
o = context.original_slide_ids
192+
expected = [o[0], o[2]]
193+
actual = [s.slide_id for s in context.slides]
194+
assert actual == expected, "expected %r, got %r" % (expected, actual)

src/pptx/oxml/presentation.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,50 @@ def add_sldId(self, rId: str) -> CT_SlideId:
6464
"""
6565
return self._add_sldId(id=self._next_id, rId=rId)
6666

67+
def insert_sldId_at(self, rId: str, idx: int) -> CT_SlideId:
68+
"""Insert a new `p:sldId` child element at position `idx`.
69+
70+
The new `p:sldId` element has its `r:id` attribute set to `rId` and
71+
receives the next available `id` value. `idx` may equal the current
72+
length to append. Raises `IndexError` if `idx` is out of range.
73+
"""
74+
if idx < 0 or idx > len(self.sldId_lst):
75+
raise IndexError("slide index out of range")
76+
new_sldId = self.add_sldId(rId)
77+
if idx < len(self.sldId_lst) - 1:
78+
target = self.sldId_lst[idx]
79+
target.addprevious(new_sldId)
80+
return new_sldId
81+
82+
def move_sldId_to(self, sldId: CT_SlideId, new_idx: int) -> None:
83+
"""Reposition `sldId` to zero-based position `new_idx` in this list.
84+
85+
`sldId` must already be a child of this element. Raises `IndexError`
86+
if `new_idx` is out of range.
87+
"""
88+
sldId_lst = self.sldId_lst
89+
if new_idx < 0 or new_idx >= len(sldId_lst):
90+
raise IndexError("slide index out of range")
91+
if sldId_lst[new_idx] is sldId:
92+
return
93+
# -- detach from current position --
94+
self.remove(sldId)
95+
# -- re-fetch list so index reflects post-removal state --
96+
sldId_lst = self.sldId_lst
97+
if new_idx >= len(sldId_lst):
98+
self.append(sldId)
99+
else:
100+
sldId_lst[new_idx].addprevious(sldId)
101+
102+
def remove_sldId(self, sldId: CT_SlideId) -> None:
103+
"""Remove `sldId` child element from this list.
104+
105+
Raises `ValueError` if `sldId` is not a child of this element.
106+
"""
107+
if sldId.getparent() is not self:
108+
raise ValueError("sldId is not a child of this sldIdLst")
109+
self.remove(sldId)
110+
67111
@property
68112
def _next_id(self) -> int:
69113
"""The next available slide ID as an `int`.

src/pptx/slide.py

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,19 @@ def slide_layout(self) -> SlideLayout:
246246
"""|SlideLayout| object this slide inherits appearance from."""
247247
return self.part.slide_layout
248248

249+
def delete(self) -> None:
250+
"""Remove this slide from its presentation.
251+
252+
Convenience alias for :meth:`Slides.remove`. After this call, the slide
253+
is no longer reachable via the presentation's `slides` collection and
254+
its part is dropped from the package on save.
255+
256+
Raises |ValueError| if this slide is not part of a presentation (e.g.
257+
the parent collection cannot be located).
258+
"""
259+
prs = self.part.package.presentation_part.presentation
260+
prs.slides.remove(self)
261+
249262

250263
class Slides(ParentedElementProxy):
251264
"""Sequence of slides belonging to an instance of |Presentation|.
@@ -277,11 +290,30 @@ def __len__(self) -> int:
277290
"""Support len() built-in function, e.g. `len(slides) == 4`."""
278291
return len(self._sldIdLst)
279292

280-
def add_slide(self, slide_layout: SlideLayout) -> Slide:
281-
"""Return a newly added slide that inherits layout from `slide_layout`."""
293+
def add_slide(self, slide_layout: SlideLayout, index: int | None = None) -> Slide:
294+
"""Return a newly added slide that inherits layout from `slide_layout`.
295+
296+
When `index` is |None| (the default), the new slide is appended to the
297+
end of the slide sequence — matching prior behavior. When `index` is
298+
an integer, the new slide is inserted at that zero-based position;
299+
`index` may equal `len(self)` to append explicitly. Negative indices
300+
are not supported; pass an explicit position. Raises |IndexError| if
301+
`index` is out of range (negative, or greater than `len(self)`).
302+
303+
Companion operations: :meth:`remove`, :meth:`move`. Cross-deck copy
304+
(``Presentation.append_from``) and ``Slide.duplicate`` are tracked
305+
under issue #11 (Phase 2/3) and not yet implemented.
306+
"""
307+
# ---validate index BEFORE creating the new SlidePart so a bad index
308+
# does not leak a partial part into the package---
309+
if index is not None and (index < 0 or index > len(self._sldIdLst)):
310+
raise IndexError("slide index out of range")
282311
rId, slide = self.part.add_slide(slide_layout)
283312
slide.shapes.clone_layout_placeholders(slide_layout)
284-
self._sldIdLst.add_sldId(rId)
313+
if index is None:
314+
self._sldIdLst.add_sldId(rId)
315+
else:
316+
self._sldIdLst.insert_sldId_at(rId, index)
285317
return slide
286318

287319
def get(self, slide_id: int, default: Slide | None = None) -> Slide | None:
@@ -304,6 +336,38 @@ def index(self, slide: Slide) -> int:
304336
return idx
305337
raise ValueError("%s is not in slide collection" % slide)
306338

339+
def move(self, slide: Slide, new_index: int) -> None:
340+
"""Reposition `slide` to zero-based position `new_index`.
341+
342+
Raises |ValueError| if `slide` is not a member of this collection,
343+
and |IndexError| if `new_index` is out of range (negative or
344+
``>= len(self)``). Section membership (`p14:sectionLst`) references
345+
slides by id, not position, so reordering is invisible to sections.
346+
"""
347+
if new_index < 0 or new_index >= len(self):
348+
raise IndexError("slide index out of range")
349+
idx = self.index(slide)
350+
sldId = self._sldIdLst.sldId_lst[idx]
351+
self._sldIdLst.move_sldId_to(sldId, new_index)
352+
353+
def remove(self, slide: Slide) -> None:
354+
"""Remove `slide` from this collection.
355+
356+
The slide's relationship is dropped from the presentation part. The
357+
underlying slide part falls out of the package on the next save.
358+
Image and other media parts referenced only by the removed slide
359+
likewise drop out — but image parts shared with surviving slides
360+
(e.g. the same picture inserted on two slides) are preserved.
361+
Raises |ValueError| if `slide` is not a member of this collection.
362+
After this call, references to `slide` are stale; behavior of method
363+
calls on the removed `Slide` instance is undefined.
364+
"""
365+
idx = self.index(slide)
366+
sldId = self._sldIdLst.sldId_lst[idx]
367+
target_rId = sldId.rId
368+
self._sldIdLst.remove_sldId(sldId)
369+
self.part.drop_rel(target_rId)
370+
307371

308372
class SlideLayout(_BaseSlide):
309373
"""Slide layout object.

tests/opc/test_package.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,17 @@ def it_constructs_custom_part_type_for_registered_content_types(self, request, p
514514
SlidePart_ = class_mock(request, "pptx.opc.package.XmlPart")
515515
SlidePart_.load.return_value = part_
516516
partname = PackURI("/ppt/slides/slide7.xml")
517+
# ---restore the original PartFactory mapping after the test so the
518+
# mock does not leak into subsequent tests that load slides
519+
original = PartFactory.part_type_for.get(CT.PML_SLIDE)
520+
521+
def _restore():
522+
if original is None:
523+
PartFactory.part_type_for.pop(CT.PML_SLIDE, None)
524+
else:
525+
PartFactory.part_type_for[CT.PML_SLIDE] = original
526+
527+
request.addfinalizer(_restore)
517528
PartFactory.part_type_for[CT.PML_SLIDE] = SlidePart_
518529

519530
part = PartFactory(partname, CT.PML_SLIDE, package_, b"blob")

0 commit comments

Comments
 (0)