feat(slides): slide CRUD Phase 1 — remove, move, indexed add_slide#33
Merged
Conversation
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
…e 2) (#34) Closes the Phase 2 sub-task of the slide-CRUD epic (#11) and the 12-year-old most-commented upstream feature request scanny#132 in this fork. API --- - `Slides.duplicate(slide, index=None)` — collection-level operation, returns a `Slide`. `index=None` defaults to `source_index + 1` (immediately after the source). Raises `ValueError` if `slide` is not a member, `IndexError` if `index` is out of range. - `Slide.duplicate(index=None)` — convenience alias delegating to `Slides.duplicate(self, index)`. Mirrors `Slide.delete()` → `Slides.remove(self)` from Phase 1. Design ------ The implementation walks the source slide's rels and decides per reltype whether to share the target part (image, media, slide-layout, slide-master, theme — package-level dedup) or deep-copy it (chart, OLE-object, embedded-package, notes-slide). For chart parts the chart XML is `copy.deepcopy`'d but the embedded xlsx workbook is *blob- copied* — the workbook is the chart's data and `<c:numCache>` values in the chart XML mirror it; deepcopy of XML would not capture the binary payload. After deepcopy, the duplicated `<p:sld>` still references the source's `rId`s. We walk every descendant element via `lxml.iter()` and substitute any attribute whose name is in the OOXML relationships namespace — catches `r:id`, `r:embed`, `r:link`, `r:pict`, `r:href` in one pass without enumerating local-names (the local-name approach silently misses `r:link` on linked images, which is the most common real-world failure mode of community recipes). Notes-slide handling addresses upstream gotcha scanny#961: a NotesSlidePart back-references its parent slide via `RT.SLIDE`. The duplicate's notes-slide MUST point at the new slide, not the source. The new helper `duplicate_notes_slide_for(src, new)` rewires the back-rel and is invoked AFTER the new slide part is registered in presentation rels. Comments parts (`RT.COMMENTS` / `RT.COMMENT_AUTHORS`) are dropped on duplicate by an explicit reltype filter — comments support is out of scope for Phase 2 of #11. A test documents the drop so the policy doesn't silently change. Test coverage ------------- - `tests/test_slide_duplicate.py` — 39 new tests across 7 describe classes covering the API surface (existence, signature, raises, index variants, alias delegation), the part-graph invariants (new unique partname / rId / sldId, layout-part shared with source, modification isolation, image-dedup invariant unchanged), the notes-slide path (own copy, back-ref rewired, isolation), the defensive XPath `r:*` check (no unmapped rIds), and round-trip through save/reopen. - `features/sld-duplicate.feature` + 7 new step impls — 4 acceptance scenarios for default-index, explicit-index, alias, and IndexError paths. Verification ------------ ``` $ python3 -m pytest tests/ -q 3125 passed in 4.02s $ python3 -m ruff format src tests 203 files left unchanged $ python3 -m ruff check src tests All checks passed! $ python3 -m behave features/ --no-color 990 scenarios passed, 0 failed, 0 skipped ``` UAT script `uat_slide_duplicate.py` (untracked at repo root per CLAUDE.md §6): builds a 3-slide deck with a textbox, a picture, and speaker notes; exercises both `Slides.duplicate` and `Slide.duplicate`; round-trips; prints structural summary (image_part_count = 2 before AND after — dedup invariant holds). Out of scope (deferred per epic split): - `Presentation.append_from` — Phase 3 (cross-deck rels). - `Presentation.sections` — Phase 4 (`<p14:sectionLst>`). Refs #11. Co-authored-by: Matthew Horoszowski <mhoroszowski@Winton.lan>
This was referenced May 7, 2026
MHoroszowski
added a commit
that referenced
this pull request
May 8, 2026
) (#36) Phase 4 of issue #11 (slide CRUD epic). Implements PowerPoint Sections support — read, write, and round-trip the `p:extLst/p:ext{uri=521415D9-…}/p14:sectionLst` extension that PowerPoint uses to organize slides into named groups in the slide pane. The fork's slide CRUD epic is now complete: Phase 1 (#33), Phase 2 (#34), Phase 3 (#35), and Phase 4 (this PR) collectively shipped duplicate / delete / reorder / copy-between-decks / sections across the originally listed sub-features. New public API -------------- - `Presentation.sections` → `_Sections` collection. - `_Sections` is sequence-like: `__len__`, `__iter__`, `__getitem__`, `index()`. Adds: * `add_section(name, after=None) -> Section` — append, or insert immediately after an existing section when `after` is given. * `remove(section)` — drop section; cleans up the wrapping `p14:sectionLst` / `p:ext` / `p:extLst` chain when the last section goes. - `Section` exposes: * `name` (read/write str) * `id` (read-only GUID-with-braces, e.g. `{ABC-…}`) * `slides` (tuple of |Slide|, in section order) * `add_slide(slide)` — assign or move slide into this section (a slide can belong to at most one section) * `remove_slide(slide)` — drop a slide's section assignment; slide remains in the presentation. Membership invariants --------------------- Section membership references slides by their stable `p:sldId/@id` integer (NOT by `r:id` and NOT by position), so `Slides.move(...)`, indexed `add_slide(...)`, and `Slides.remove(...)` preserve assignment without any extra plumbing. Removed slides become orphan ids in the section's `p14:sldIdLst`; `Section.slides` silently skips them on read but the XML round-trips them untouched (deliberate — matching python-pptx's "preserve foreign data" doctrine). PowerPoint compatibility ------------------------ - Empty sections emit `<p14:sldIdLst/>` so all PowerPoint versions treat them consistently (some interpret an omitted `sldIdLst` as "all unsectioned slides," which is not what we mean). - Section ids generated as `{<UPPER-CASE-GUID>}` matching PowerPoint's wire shape. - Foreign `p:extLst/p:ext` siblings (`p15:*`, modification tracking, etc.) round-trip untouched — the section ext lives alongside, not in place of. Internal additions ------------------ - New `pptx/sections.py` module hosting `Section` and `_Sections` proxy classes. - `pptx/oxml/presentation.py` extended with `CT_PresentationExtensionList`, `CT_PresentationExtension`, `CT_SectionList`, `CT_Section`, `CT_SectionSlideIdList`, `CT_SectionSlideId`, plus `SECTION_LIST_EXT_URI` constant and `CT_Presentation.{section_list, get_or_add_section_list, remove_section_list}` helpers. - `pptx/oxml/ns.py` registers the `p14` prefix (`http://schemas.microsoft.com/office/powerpoint/2010/main`). - `pptx/oxml/__init__.py` registers the new element classes. - `p:extLst` added as a successor entry on the existing ZeroOrOne `sldMasterIdLst`/`sldIdLst`/`sldSz` slots so insert ordering is correct. Tangential test-infra fix ------------------------- `tests/unitutil/cxml.py` namespace-prefix grammar widened from `Word(alphas)` to `Word(alphas, alphanums)` so test fixtures can address `p14:`, `w14:`, `o15:`, etc. Local-name grammar already supported alphanums. Test coverage ------------- - 56 new unit tests in `tests/test_sections.py` covering oxml helpers, the `Section`/`_Sections` proxies, and 8 round-trip integration tests (no-sections baseline, names + membership, GUID preservation, membership-survives-move, removal pruning, orphan preservation, empty-section, unicode/XML-special name, unsectioned-on-section-remove, sibling-ext preservation). - 5 new behave scenarios in `features/sld-sections.feature` (default empty, add, slide membership, move-preserves-membership, remove cleans up extLst). - New `uat_slide_sections.py` (untracked per repo §6) builds a 6-slide / 3-section deck, demonstrates membership preservation through a slide move, and prints a structural read-back. Verification ------------ ``` $ python3 -m pytest tests/ -q | tail -3 3222 passed in 4.33s $ ruff check src tests | tail -3 All checks passed! $ python3 -m behave features/ --no-color | tail -3 999 scenarios passed, 0 failed, 0 skipped 3000 steps passed, 0 failed, 0 skipped ``` Closes #11 Co-authored-by: Matthew Horoszowski <mhoroszowski@Winton.lan>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of slide CRUD epic (#11) — in-deck CRUD only.
Adds a coherent slice of programmatic deck assembly that today requires lxml fallbacks:
Slides.remove(slide)— drop a slide from the deck; underlying part falls out on save; image parts shared with surviving slides are preserved (package-level dedup invariant holds).Slide.delete()— convenience alias delegating toSlides.remove(self).Slides.move(slide, new_index)— reposition a slide in the deck. Section membership (<p14:sectionLst>) references by id, so reordering is invisible to sections.add_slide(slide_layout, index=None)— backwards-compatible:index=Nonekeeps appending;index=Ninserts the new slide at the zero-based position N. Index validation runs before the newSlidePartis created so a bad index does not leak a partial part into the package.API style mirrors
SlideLayouts.remove(slide_layout)already in this codebase rather than upstream PR #1029'sremove_slide(slide_id)(less Pythonic — passes an opaque integer rather than the object).Out of scope for this PR (deferred per epic split):
Slide.duplicate→ Phase 2 (next branch — needs full part deep-copy + image dedup)Presentation.append_from→ Phase 3 (cross-deck rels resolution)Presentation.sections→ Phase 4 (<p14:sectionLst>namespace)OXML helpers added
oxml/presentation.py::insert_sldId_at(rId, idx)— insert an<p:sldId>at a specific position.oxml/presentation.py::move_sldId_to(sldId, new_idx)— reorder viaaddprevious.Existing
add_sldId(rId)(append) and_next_idallocator reused.Tests
tests/test_slide_crud.pycovering all four API entry points: argument validation, ValueError/IndexError raises, sldId/rels invariants, and round-trip preservation.features/sld-crud.feature(3 scenarios — remove, move, indexed add) wired throughfeatures/steps/slides.py.UAT
uat_slide_crud.pyat the repo root builds a 5-slide deck, exercises remove / move / indexed add, and prints round-trip read-back. Maintainer signoff received.Closes the in-deck-CRUD portion of #11. Refs #11.