From fcf7121b696c9ba37ee31af9ac0ed1bc45b8373a Mon Sep 17 00:00:00 2001 From: Matthew Horoszowski Date: Fri, 8 May 2026 13:41:24 -0400 Subject: [PATCH] feat: Font.color non-mutation + UTC-aware datetimes + Shapes.by_name (Modernization Phase 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: https://github.com/MHoroszowski/python-pptx/issues/29 (Phase 2) Phase 1 (PR #39) shipped PathLike + PERCENT_40 typo + Slide.background fix. This PR closes three nagging upstream tickets in a single bundle, all covered by issue #29's "bug fixes that pollute every other epic if left unfixed" group plus the most-cited shape-lookup ergonomic from the API ergonomics group. Bug fixes - `Font.color` getter is now non-mutating. Prior implementation called `self.fill.solid()` on every read, inserting `` into the run's `` element — meaning *reading* a font's color silently modified the document. New implementation returns a `_LazyFontColorFormat` proxy that mirrors `ColorFormat`'s public surface; reads (`type`, `rgb`, `theme_color`, `brightness`, `transparency`) return |None| / inherit values without writing; the setter path materializes `` lazily on first write and delegates to the real `ColorFormat`. Closes scanny/python-pptx#1111 and scanny/python-pptx#1074. - W3CDTF datetime parser returns tz-aware datetimes when the source string carries timezone information. `Z` suffix → UTC; numeric offsets like `-08:00` or `+05:30` → fixed-offset `datetime.timezone`. Strings with no offset (year-only, year-month, year-month-day, or bare timestamp) continue to return naive datetimes — we don't assume a timezone where none was given. The setter accepts both naive and tz-aware inputs; tz-aware inputs are converted to UTC via `astimezone(timezone.utc)` before serialization so the on-disk W3CDTF form always uses the canonical `YYYY-MM-DDTHH:MM:SSZ` shape. Closes scanny/python-pptx#957. Sign-convention fix: the prior `_offset_dt` had `sign_factor = -1 if sign == "+" else 1` (inverted vs. POSIX/W3CDTF convention) and *adjusted the clock values*; the new `_tzinfo_from_offset_str` uses `sign_factor = 1 if sign == "+" else -1` and returns a `tzinfo` instance, leaving the clock values intact. End-to-end behavior is preserved when callers convert via `astimezone(utc)` — `'2024-01-15 T10:30:00-08:00'` still represents the same instant (18:30 UTC). API ergonomics - `_BaseShapes.by_name(name)` lookup helper. Returns the first shape in document order whose `.name` matches `name` (case-sensitive, matching PowerPoint's behavior). Raises `KeyError` with a clear message on miss. Defined on `_BaseShapes` so it inherits to `SlideShapes`, `SlideLayoutShapes`, `SlideMasterShapes`, etc. Closes scanny/python-pptx#798, scanny/python-pptx#309, and scanny/python-pptx#532. Behavior changes (intentional, documented in the PR body) - Reading `font.color` no longer inserts ``. Code that relied on the mutation as a side-effect needs to call `font.fill.solid()` explicitly. - Reading `core_properties.created` / `last_printed` / `modified` on a document whose XML carries `Z` or numeric offset markers now returns a tz-aware datetime. Naive-input expectations must update. Tests - 27 new pytest cases in `tests/test_modernization_phase2.py` covering byte-stable XML on Font.color reads, lazy materialization on first set, datetime parser tz-awareness across all five W3CDTF input forms, round-trip through save/reload, and the three by_name paths (hit / miss / case-sensitivity / multi-match / inheritance to layouts and masters). Two existing test fixtures updated to expect tz-aware values where the input strings carry `Z`. Full pytest: `3453 passed`. - 5 new behave scenarios in `features/modernization-phase2.feature` (Font.color non-mutation byte-diff, lazy materialization, tz-aware round-trip, by_name match, by_name miss). Existing `features/steps/coreprops.py` updated to use tz-aware datetimes for the now-tz-aware reload values. Full behave: `1041 scenarios passed, 0 failed`. - Ruff: `ruff check src tests` → All checks passed; `ruff format --check` → no diff. Skipped from issue #29 Phase 2 (deferred or no-op) - `collections.abc` import path migration (would close scanny#771): already complete in this fork; verified via grep across `src/`. - `iter_leaf_shapes`, `Mapping` ABC, `find_by_xpath`, selection-pane order listing: deferred to a future Phase 4 to keep Phase 2 focused on bug fixes. Refs #29 --- features/modernization-phase2.feature | 33 +++ features/steps/coreprops.py | 17 +- features/steps/modernization_phase2.py | 106 +++++++++ src/pptx/oxml/coreprops.py | 62 +++-- src/pptx/shapes/shapetree.py | 17 ++ src/pptx/text/text.py | 106 ++++++++- tests/parts/test_coreprops.py | 17 +- tests/test_modernization_phase2.py | 300 +++++++++++++++++++++++++ tests/text/test_text.py | 13 +- 9 files changed, 634 insertions(+), 37 deletions(-) create mode 100644 features/modernization-phase2.feature create mode 100644 features/steps/modernization_phase2.py create mode 100644 tests/test_modernization_phase2.py diff --git a/features/modernization-phase2.feature b/features/modernization-phase2.feature new file mode 100644 index 000000000..59d489980 --- /dev/null +++ b/features/modernization-phase2.feature @@ -0,0 +1,33 @@ +Feature: Modernization & Ergonomics Phase 2 — bug fixes + by_name + In order to inspect fonts without polluting the document, round-trip datetimes faithfully, and look up shapes by name + As a developer using python-pptx + I need a non-mutating Font.color getter, tz-aware core-property datetimes, and Shapes.by_name(name) + + + Scenario: Reading font.color does not insert a:solidFill + Given a fresh slide with a title placeholder + When I read run.font.color.rgb on an unstyled run + Then the underlying rPr XML is unchanged from before the access + + + Scenario: Setting font.color.rgb materializes a:solidFill lazily + Given a fresh slide with a title placeholder + When I set run.font.color.rgb to RGBColor(0xFF, 0x00, 0x00) + Then the underlying rPr now contains an a:solidFill child + And run.font.color.rgb reads back as FF0000 + + + Scenario: Tz-aware core_properties.created round-trips faithfully + Given a fresh presentation for core-property datetimes + When I set core_properties.created to a tz-aware UTC datetime + Then the reloaded core_properties.created is tz-aware + + + Scenario: shapes.by_name returns the matching shape + Given a fresh slide with a title placeholder + Then shapes.by_name("Title 1") returns the title shape + + + Scenario: shapes.by_name raises KeyError on miss + Given a fresh slide with a title placeholder + Then shapes.by_name("Bogus") raises KeyError diff --git a/features/steps/coreprops.py b/features/steps/coreprops.py index 9989c2e01..defa8b4fe 100644 --- a/features/steps/coreprops.py +++ b/features/steps/coreprops.py @@ -2,7 +2,7 @@ from __future__ import annotations -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from behave import given, then, when from helpers import no_core_props_pptx_path, saved_pptx_path @@ -28,18 +28,21 @@ def step_when_open_presentation_with_no_core_props_part(context): @when("I set the core properties to valid values") def step_when_set_core_doc_props_to_valid_values(context): + # ---issue #29 Phase 2: datetimes round-trip as tz-aware UTC values, so + # ---the input fixture is already tz-aware UTC for an apples-to-apples + # ---comparison with the reloaded value. context.propvals = ( ("author", "Creator"), ("category", "Category"), ("comments", "Description"), ("content_status", "Content Status"), - ("created", datetime(2013, 6, 15, 12, 34, 56)), + ("created", datetime(2013, 6, 15, 12, 34, 56, tzinfo=timezone.utc)), ("identifier", "Identifier"), ("keywords", "key; word; keyword"), ("language", "Language"), ("last_modified_by", "Last Modified By"), - ("last_printed", datetime(2013, 6, 15, 12, 34, 56)), - ("modified", datetime(2013, 6, 15, 12, 34, 56)), + ("last_printed", datetime(2013, 6, 15, 12, 34, 56, tzinfo=timezone.utc)), + ("modified", datetime(2013, 6, 15, 12, 34, 56, tzinfo=timezone.utc)), ("revision", 9), ("subject", "Subject"), # --- exercise unicode-text case for Python 2.7 --- @@ -60,8 +63,10 @@ def step_then_a_core_props_part_with_def_vals_is_added(context): assert core_props.last_modified_by == "python-pptx" assert core_props.revision == 1 # core_props.modified only stores time with seconds resolution, so - # comparison needs to be a little loose (within two seconds) - modified_timedelta = datetime.utcnow() - core_props.modified + # comparison needs to be a little loose (within two seconds). Issue #29 + # Phase 2 makes the parser return a tz-aware datetime; use a tz-aware + # `now()` here so the subtraction is well-typed. + modified_timedelta = datetime.now(timezone.utc) - core_props.modified max_expected_timedelta = timedelta(seconds=2) assert modified_timedelta < max_expected_timedelta diff --git a/features/steps/modernization_phase2.py b/features/steps/modernization_phase2.py new file mode 100644 index 000000000..6f49d99c6 --- /dev/null +++ b/features/steps/modernization_phase2.py @@ -0,0 +1,106 @@ +"""Gherkin steps for Modernization Phase 2 (issue #29).""" + +from __future__ import annotations + +import datetime as dt +import io + +import pytest +from behave import given, then, when +from lxml import etree + +from pptx import Presentation +from pptx.dml.color import RGBColor + + +# given =================================================== + + +@given("a fresh slide with a title placeholder") +def given_a_fresh_slide_with_title(context): + prs = Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[1]) + context.prs = prs + context.slide = slide + + +@given("a fresh presentation for core-property datetimes") +def given_a_fresh_presentation_for_core_props(context): + context.prs = Presentation() + + +# when ==================================================== + + +@when("I read run.font.color.rgb on an unstyled run") +def when_read_font_color_unstyled(context): + tf = context.slide.shapes.title.text_frame + run = tf.paragraphs[0].add_run() + run.text = "x" + rPr = run._r.get_or_add_rPr() + context.run = run + context.rPr = rPr + context.before_xml = etree.tostring(rPr) + _ = run.font.color.rgb + + +@when("I set run.font.color.rgb to RGBColor(0xFF, 0x00, 0x00)") +def when_set_font_color_red(context): + tf = context.slide.shapes.title.text_frame + run = tf.paragraphs[0].add_run() + run.text = "x" + context.run = run + context.rPr = run._r.get_or_add_rPr() + run.font.color.rgb = RGBColor(0xFF, 0x00, 0x00) + + +@when("I set core_properties.created to a tz-aware UTC datetime") +def when_set_created_utc(context): + context.target_dt = dt.datetime(2024, 7, 4, 17, 0, 0, tzinfo=dt.timezone.utc) + context.prs.core_properties.created = context.target_dt + + +# the "save and reload via stream" step is shared with tbl_styles.py + + +# then ==================================================== + + +@then("the underlying rPr XML is unchanged from before the access") +def then_rPr_unchanged(context): + after = etree.tostring(context.rPr) + assert context.before_xml == after, (context.before_xml, after) + + +@then("the underlying rPr now contains an a:solidFill child") +def then_rPr_has_solidFill(context): + ns = "{http://schemas.openxmlformats.org/drawingml/2006/main}" + assert context.rPr.find("%ssolidFill" % ns) is not None + + +@then("run.font.color.rgb reads back as FF0000") +def then_font_color_rgb_reads_FF0000(context): + assert context.run.font.color.rgb == RGBColor(0xFF, 0x00, 0x00) + + +@then("the reloaded core_properties.created is tz-aware") +def then_reloaded_created_is_tzaware(context): + buf = io.BytesIO() + context.prs.save(buf) + buf.seek(0) + prs2 = Presentation(buf) + reloaded = prs2.core_properties.created + assert reloaded.tzinfo is not None + assert reloaded == context.target_dt + + +@then('shapes.by_name("Title 1") returns the title shape') +def then_by_name_returns_title(context): + sh = context.slide.shapes.by_name("Title 1") + assert sh.name == "Title 1" + + +@then('shapes.by_name("Bogus") raises KeyError') +def then_by_name_raises_keyerror(context): + with pytest.raises(KeyError): + context.slide.shapes.by_name("Bogus") diff --git a/src/pptx/oxml/coreprops.py b/src/pptx/oxml/coreprops.py index de6b26b24..14892f044 100644 --- a/src/pptx/oxml/coreprops.py +++ b/src/pptx/oxml/coreprops.py @@ -210,34 +210,44 @@ def _get_or_add(self, prop_name: str): return element @classmethod - def _offset_dt(cls, datetime: dt.datetime, offset_str: str): - """Return |datetime| instance offset from `datetime` by offset specified in `offset_str`. - - `offset_str` is a string like `'-07:00'`. - """ + def _tzinfo_from_offset_str(cls, offset_str: str) -> dt.timezone: + """Return a :class:`datetime.timezone` parsed from a W3CDTF offset like '-08:00'.""" match = cls._offset_pattern.match(offset_str) if match is None: raise ValueError(f"{repr(offset_str)} is not a valid offset string") sign, hours_str, minutes_str = match.groups() - sign_factor = -1 if sign == "+" else 1 + sign_factor = 1 if sign == "+" else -1 hours = int(hours_str) * sign_factor minutes = int(minutes_str) * sign_factor - td = dt.timedelta(hours=hours, minutes=minutes) - return datetime + td + return dt.timezone(dt.timedelta(hours=hours, minutes=minutes)) _offset_pattern = re.compile(r"([+-])(\d\d):(\d\d)") @classmethod def _parse_W3CDTF_to_datetime(cls, w3cdtf_str: str) -> dt.datetime: - # valid W3CDTF date cases: - # yyyy e.g. '2003' - # yyyy-mm e.g. '2003-12' - # yyyy-mm-dd e.g. '2003-12-31' - # UTC timezone e.g. '2003-12-31T10:14:55Z' - # numeric timezone e.g. '2003-12-31T10:14:55-08:00' + """Parse a W3CDTF string into a :class:`datetime.datetime`. + + Returns a tz-aware datetime when the input string carries timezone + information — ``Z`` suffix maps to UTC; numeric offsets like + ``-08:00`` map to a fixed-offset :class:`datetime.timezone`. When the + input has no timezone marker (year-only, year-month, year-month-day, + or a bare timestamp), the returned datetime is naive — callers + should not assume any specific timezone for naive inputs. + + Closes scanny/python-pptx#957 — the prior implementation always + returned a naive datetime, even for strings that explicitly carried + timezone information. + + valid W3CDTF date cases: + - yyyy e.g. '2003' + - yyyy-mm e.g. '2003-12' + - yyyy-mm-dd e.g. '2003-12-31' + - UTC timezone e.g. '2003-12-31T10:14:55Z' + - numeric timezone e.g. '2003-12-31T10:14:55-08:00' + """ templates = ("%Y-%m-%dT%H:%M:%S", "%Y-%m-%d", "%Y-%m", "%Y") - # strptime isn't smart enough to parse literal timezone offsets like - # '-07:30', so we have to do it ourselves + # ---strptime can't parse literal timezone offsets like '-07:30', so + # ---we strip the offset and add tzinfo ourselves below. parseable_part = w3cdtf_str[:19] offset_str = w3cdtf_str[19:] timestamp = None @@ -249,15 +259,31 @@ def _parse_W3CDTF_to_datetime(cls, w3cdtf_str: str) -> dt.datetime: if timestamp is None: tmpl = "could not parse W3CDTF datetime string '%s'" raise ValueError(tmpl % w3cdtf_str) + # ---'Z' means UTC--- + if offset_str == "Z": + return timestamp.replace(tzinfo=dt.timezone.utc) + # ---numeric offset like '-08:00'--- if len(offset_str) == 6: - return cls._offset_dt(timestamp, offset_str) + return timestamp.replace(tzinfo=cls._tzinfo_from_offset_str(offset_str)) + # ---no timezone marker; return naive (don't assume UTC)--- return timestamp def _set_element_datetime(self, prop_name: str, value: dt.datetime) -> None: - """Set date/time value of child element having `prop_name` to `value`.""" + """Set date/time value of child element having `prop_name` to `value`. + + Accepts both naive and tz-aware datetimes. Tz-aware inputs are + converted to UTC before serialization so the on-disk W3CDTF form + always uses the canonical ``YYYY-MM-DDTHH:MM:SSZ`` shape. Naive + inputs are written as-is (with the trailing ``Z`` suffix indicating + UTC) — callers passing naive datetimes are responsible for the + timezone interpretation. + """ if not isinstance(value, dt.datetime): # pyright: ignore[reportUnnecessaryIsInstance] tmpl = "property requires object, got %s" raise ValueError(tmpl % type(value)) + # ---tz-aware -> normalize to UTC before serializing--- + if value.tzinfo is not None: + value = value.astimezone(dt.timezone.utc).replace(tzinfo=None) element = self._get_or_add(prop_name) dt_str = value.strftime("%Y-%m-%dT%H:%M:%SZ") element.text = dt_str diff --git a/src/pptx/shapes/shapetree.py b/src/pptx/shapes/shapetree.py index ca088dd48..6b5f2f3ad 100644 --- a/src/pptx/shapes/shapetree.py +++ b/src/pptx/shapes/shapetree.py @@ -108,6 +108,23 @@ def __len__(self) -> int: shape_elms = list(self._iter_member_elms()) return len(shape_elms) + def by_name(self, name: str) -> BaseShape: + """Return the first shape in this collection whose `.name` equals `name`. + + Lookup is case-sensitive, matching PowerPoint's own behavior. When + multiple shapes share the same name (uncommon but possible — + PowerPoint does not enforce uniqueness), the first match in + document order is returned. Raises |KeyError| with a clear message + if no match is found. + + Closes scanny/python-pptx#798, scanny/python-pptx#309, and + scanny/python-pptx#532. + """ + for shape in self: + if shape.name == name: + return shape + raise KeyError("no shape named %r in this collection" % name) + def clone_placeholder(self, placeholder: LayoutPlaceholder) -> None: """Add a new placeholder shape based on `placeholder`.""" sp = placeholder.element diff --git a/src/pptx/text/text.py b/src/pptx/text/text.py index bb65ad65f..62b1e77dd 100644 --- a/src/pptx/text/text.py +++ b/src/pptx/text/text.py @@ -309,11 +309,18 @@ def bold(self, value: bool | None): self._rPr.b = value @lazyproperty - def color(self) -> ColorFormat: - """The |ColorFormat| instance that provides access to the color settings for this font.""" - if self.fill.type != MSO_FILL.SOLID: - self.fill.solid() - return self.fill.fore_color + def color(self) -> "_LazyFontColorFormat": + """The |ColorFormat| instance that provides access to the color settings for this font. + + Reading from the returned object on a font with no `` does not modify + the underlying XML — `font.color.type` and `font.color.rgb` simply return |None|. + Setting any color property (`rgb`, `theme_color`, `brightness`) materializes + `` lazily on first write, then delegates to the real |ColorFormat|. + + Closes scanny/python-pptx#1111 and #1074 — the prior implementation called + ``self.fill.solid()`` on every read, mutating the document on access. + """ + return _LazyFontColorFormat(self) @lazyproperty def fill(self) -> FillFormat: @@ -428,6 +435,95 @@ def underline(self, value: bool | MSO_TEXT_UNDERLINE_TYPE | None): self._element.u = value +class _LazyFontColorFormat: + """ColorFormat-shaped proxy that defers `` creation until first SET. + + Wraps a |Font| instance. On reads (``type``, ``rgb``, ``theme_color``, + ``brightness``, ``transparency``), if the font has no solid fill the proxy + returns |None| / inherit values without modifying the XML. On writes, + materializes ```` via ``font.fill.solid()`` and delegates to the + real |ColorFormat|. + + Fixes scanny/python-pptx#1111 and #1074 — the prior `Font.color` getter called + `self.fill.solid()` unconditionally on every read. + """ + + def __init__(self, font: "Font"): + self._font = font + + # ---internal helpers --------------------------------------------------- + + def _real_or_none(self) -> "ColorFormat | None": + """Return the real ColorFormat over `` if present, else |None|. + + Read path. Does NOT mutate the underlying XML. + """ + if self._font.fill.type == MSO_FILL.SOLID: + return self._font.fill.fore_color + return None + + def _real_mutating(self) -> "ColorFormat": + """Materialize `` on the font and return its real ColorFormat. + + Write path. Mutates the XML on first call by inserting `` + when not already present. + """ + if self._font.fill.type != MSO_FILL.SOLID: + self._font.fill.solid() + return self._font.fill.fore_color + + # ---public API mirroring ColorFormat ---------------------------------- + + @property + def type(self): + real = self._real_or_none() + return real.type if real is not None else None + + @property + def rgb(self): + real = self._real_or_none() + return real.rgb if real is not None else None + + @rgb.setter + def rgb(self, value): + self._real_mutating().rgb = value + + @property + def theme_color(self): + # ---no fill = "inheriting from style", which is None. + # ---NOT_THEME_COLOR is reserved for "solidFill present but no + # ---schemeClr child" — i.e. the explicit-RGB case. Conflating the + # ---two would let a round-trip read/write break inheritance. + real = self._real_or_none() + return real.theme_color if real is not None else None + + @theme_color.setter + def theme_color(self, value): + self._real_mutating().theme_color = value + + @property + def brightness(self): + # ---no fill = inherit; return None. 0.0 is a real settable value + # ---meaning "no brightness adjustment", not the inherit signal. + real = self._real_or_none() + return real.brightness if real is not None else None + + @brightness.setter + def brightness(self, value): + self._real_mutating().brightness = value + + @property + def transparency(self): + # ---no fill = inherit; return None. 0.0 is a real settable value + # ---meaning "fully opaque", not the inherit signal. + real = self._real_or_none() + return real.transparency if real is not None else None + + @transparency.setter + def transparency(self, value): + self._real_mutating().transparency = value + + class _Hyperlink(Subshape): """Text run hyperlink object. diff --git a/tests/parts/test_coreprops.py b/tests/parts/test_coreprops.py index 0983218e4..1107c4050 100644 --- a/tests/parts/test_coreprops.py +++ b/tests/parts/test_coreprops.py @@ -64,8 +64,12 @@ def it_can_change_the_string_property_values(self, prop_name: str, tagname: str, @pytest.mark.parametrize( ("prop_name", "expected_value"), [ - ("created", dt.datetime(2012, 11, 17, 16, 37, 40)), - ("last_printed", dt.datetime(2014, 6, 4, 4, 28)), + # ---fixture XML uses 'Z' suffix on these dates, so the parser + # ---returns tz-aware datetimes (issue #29 Phase 2 fix). Naive + # ---values would be returned only for input strings without + # ---a timezone marker. + ("created", dt.datetime(2012, 11, 17, 16, 37, 40, tzinfo=dt.timezone.utc)), + ("last_printed", dt.datetime(2014, 6, 4, 4, 28, tzinfo=dt.timezone.utc)), ("modified", None), ], ) @@ -145,10 +149,11 @@ def it_can_construct_a_default_core_props(self): assert core_props.revision == 1 assert core_props.modified is not None # core_props.modified only stores time with seconds resolution, so - # comparison needs to be a little loose (within two seconds) - modified_timedelta = ( - dt.datetime.now(dt.timezone.utc).replace(tzinfo=None) - core_props.modified - ) + # comparison needs to be a little loose (within two seconds). Issue + # #29 Phase 2 makes the parser return a tz-aware datetime when the + # source string carries a 'Z' suffix; the default-constructor path + # writes the modified field with 'Z', so the reload returns UTC. + modified_timedelta = dt.datetime.now(dt.timezone.utc) - core_props.modified max_expected_timedelta = dt.timedelta(seconds=2) assert modified_timedelta < max_expected_timedelta diff --git a/tests/test_modernization_phase2.py b/tests/test_modernization_phase2.py new file mode 100644 index 000000000..65ad14fee --- /dev/null +++ b/tests/test_modernization_phase2.py @@ -0,0 +1,300 @@ +# pyright: reportPrivateUsage=false + +"""Unit-test suite for Modernization Phase 2 — bug fixes + by_name ergonomics. + +Covers: + +- |Font|.color getter is non-mutating (closes scanny/python-pptx#1111, #1074): + reading `font.color.rgb` / `.type` / etc. on an unstyled run does NOT + insert ```` into the underlying XML. Setting + `font.color.rgb = ...` still works (lazy materialization on first SET). +- W3CDTF datetime parser returns tz-aware datetimes when the source string + carries timezone info, naive datetimes when it doesn't. Setter + normalizes tz-aware inputs to UTC before serialization. Closes + scanny/python-pptx#957. +- |_BaseShapes|.by_name(name) lookup helper returns the first shape with + matching name or raises KeyError. Closes scanny/python-pptx#798, + scanny/python-pptx#309, scanny/python-pptx#532. +- Anti-criteria: existing `font.color.rgb = ...` setter unchanged; existing + Phase-1 fixes still in place. + +Issue: https://github.com/MHoroszowski/python-pptx/issues/29 (Phase 2). +""" + +from __future__ import annotations + +import datetime as dt + +import pytest +from lxml import etree + +from pptx import Presentation +from pptx.dml.color import RGBColor +from pptx.enum.dml import MSO_COLOR_TYPE, MSO_THEME_COLOR +from pptx.oxml.coreprops import CT_CoreProperties +from pptx.text.text import _LazyFontColorFormat +from pptx.util import Inches + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +def _make_run(): + prs = Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[1]) + tf = slide.shapes.title.text_frame + run = tf.paragraphs[0].add_run() + run.text = "x" + return prs, run + + +@pytest.fixture +def run_fixture(): + _, r = _make_run() + return r + + +@pytest.fixture +def slide_fixture(): + prs = Presentation() + slide = prs.slides.add_slide(prs.slide_layouts[1]) + return slide + + +# --------------------------------------------------------------------------- +# Font.color non-mutation (scanny#1111, #1074) +# --------------------------------------------------------------------------- + + +class DescribeFontColor_NonMutation(object): + """Reading `font.color` properties does NOT modify the underlying XML.""" + + def it_does_not_mutate_rPr_on_color_property_access(self, run_fixture): + rPr = run_fixture._r.get_or_add_rPr() + before = etree.tostring(rPr) + + _ = run_fixture.font.color # property access alone + + after = etree.tostring(rPr) + assert before == after + + def it_does_not_mutate_rPr_on_reading_color_type(self, run_fixture): + rPr = run_fixture._r.get_or_add_rPr() + before = etree.tostring(rPr) + + _ = run_fixture.font.color.type + + after = etree.tostring(rPr) + assert before == after + + def it_does_not_mutate_rPr_on_reading_color_rgb(self, run_fixture): + rPr = run_fixture._r.get_or_add_rPr() + before = etree.tostring(rPr) + + _ = run_fixture.font.color.rgb + + after = etree.tostring(rPr) + assert before == after + + def it_returns_None_for_color_type_on_unstyled_run(self, run_fixture): + assert run_fixture.font.color.type is None + + def it_returns_None_for_color_rgb_on_unstyled_run(self, run_fixture): + assert run_fixture.font.color.rgb is None + + def it_returns_a_LazyFontColorFormat_proxy(self, run_fixture): + assert isinstance(run_fixture.font.color, _LazyFontColorFormat) + + def it_materializes_solidFill_on_first_rgb_setter_call(self, run_fixture): + run_fixture.font.color.rgb = RGBColor(0xFF, 0x00, 0x00) + + rPr = run_fixture._r.get_or_add_rPr() + # ---a:solidFill should now exist as a child--- + ns = "{http://schemas.openxmlformats.org/drawingml/2006/main}" + assert rPr.find("%ssolidFill" % ns) is not None + + def it_round_trips_rgb_after_lazy_materialization(self, run_fixture): + run_fixture.font.color.rgb = RGBColor(0xAB, 0xCD, 0xEF) + + assert run_fixture.font.color.rgb == RGBColor(0xAB, 0xCD, 0xEF) + assert run_fixture.font.color.type == MSO_COLOR_TYPE.RGB + + def it_materializes_solidFill_on_theme_color_setter(self, run_fixture): + run_fixture.font.color.theme_color = MSO_THEME_COLOR.ACCENT_1 + + assert run_fixture.font.color.theme_color == MSO_THEME_COLOR.ACCENT_1 + assert run_fixture.font.color.type == MSO_COLOR_TYPE.SCHEME + + def it_returns_None_for_theme_color_on_unstyled_run(self, run_fixture): + # ---no solidFill = inherit from style; None is the right signal. + # ---NOT_THEME_COLOR is reserved for the explicit "solidFill exists, + # ---no schemeClr" case — conflating them would break inheritance + # ---if a caller reads then writes back the value. + assert run_fixture.font.color.theme_color is None + + def it_returns_None_for_brightness_on_unstyled_run(self, run_fixture): + # ---0.0 is a real settable "no brightness adjustment" value, so + # ---None must be the inherit signal on a fillless run. + assert run_fixture.font.color.brightness is None + + def it_returns_None_for_transparency_on_unstyled_run(self, run_fixture): + # ---0.0 means "fully opaque" — also a real settable value. + assert run_fixture.font.color.transparency is None + + def it_keeps_existing_solidFill_runs_unchanged(self, run_fixture): + # ---first set establishes a solidFill--- + run_fixture.font.color.rgb = RGBColor(0x00, 0xFF, 0x00) + rPr = run_fixture._r.get_or_add_rPr() + before = etree.tostring(rPr) + + # ---reading after first set should also be byte-stable--- + _ = run_fixture.font.color.rgb + _ = run_fixture.font.color.type + + after = etree.tostring(rPr) + assert before == after + + +# --------------------------------------------------------------------------- +# UTC-aware datetime parser/setter (scanny#957) +# --------------------------------------------------------------------------- + + +class DescribeW3CDTF_DateTime(object): + """`_parse_W3CDTF_to_datetime` returns tz-aware datetimes when source carries tz info.""" + + def it_returns_utc_for_Z_suffix(self): + result = CT_CoreProperties._parse_W3CDTF_to_datetime("2024-01-15T10:30:00Z") + assert result.tzinfo == dt.timezone.utc + + def it_returns_fixed_offset_for_negative_offset(self): + result = CT_CoreProperties._parse_W3CDTF_to_datetime("2024-01-15T10:30:00-08:00") + assert result.tzinfo == dt.timezone(dt.timedelta(hours=-8)) + + def it_returns_fixed_offset_for_positive_offset(self): + result = CT_CoreProperties._parse_W3CDTF_to_datetime("2024-01-15T10:30:00+05:30") + assert result.tzinfo == dt.timezone(dt.timedelta(hours=5, minutes=30)) + + def it_returns_naive_datetime_when_source_has_no_offset(self): + result = CT_CoreProperties._parse_W3CDTF_to_datetime("2024-01-15T10:30:00") + assert result.tzinfo is None + + def it_returns_naive_for_date_only_strings(self): + result = CT_CoreProperties._parse_W3CDTF_to_datetime("2024-01-15") + assert result.tzinfo is None + assert result == dt.datetime(2024, 1, 15) + + def it_preserves_the_correct_instant_with_offset(self): + # ---10:30 PST = 18:30 UTC--- + result = CT_CoreProperties._parse_W3CDTF_to_datetime("2024-01-15T10:30:00-08:00") + as_utc = result.astimezone(dt.timezone.utc) + assert as_utc.hour == 18 + assert as_utc.minute == 30 + + def it_round_trips_through_save_and_reload(self, tmp_path): + prs = Presentation() + # ---tz-aware input: PDT (UTC-7); 10:00 PDT = 17:00 UTC--- + prs.core_properties.created = dt.datetime( + 2024, 7, 4, 10, 0, 0, tzinfo=dt.timezone(dt.timedelta(hours=-7)) + ) + out = tmp_path / "rt.pptx" + prs.save(out) + + prs2 = Presentation(out) + reloaded = prs2.core_properties.created + # ---written as Z (UTC); reloaded value represents 17:00 UTC--- + assert reloaded.tzinfo == dt.timezone.utc + assert reloaded.hour == 17 + assert reloaded.minute == 0 + + def it_accepts_naive_datetimes_for_backwards_compat(self, tmp_path): + prs = Presentation() + # ---naive datetime; written as Z (treated as UTC by convention)--- + prs.core_properties.created = dt.datetime(2024, 1, 15, 10, 30, 0) + out = tmp_path / "rt2.pptx" + prs.save(out) + + prs2 = Presentation(out) + reloaded = prs2.core_properties.created + # ---reloaded value is tz-aware (Z parsed)--- + assert reloaded.tzinfo == dt.timezone.utc + assert reloaded.hour == 10 + + +# --------------------------------------------------------------------------- +# Shapes.by_name (scanny#798, #309, #532) +# --------------------------------------------------------------------------- + + +class DescribeShapes_by_name(object): + """`shapes.by_name(name)` lookup helper.""" + + def it_returns_the_shape_with_matching_name(self, slide_fixture): + # ---the title shape on a Title+Content layout is named 'Title 1'--- + title = slide_fixture.shapes.by_name("Title 1") + assert title.name == "Title 1" + + def it_raises_KeyError_on_no_match(self, slide_fixture): + with pytest.raises(KeyError) as excinfo: + slide_fixture.shapes.by_name("Bogus") + assert "Bogus" in str(excinfo.value) + + def it_is_case_sensitive(self, slide_fixture): + with pytest.raises(KeyError): + slide_fixture.shapes.by_name("title 1") # lowercase t + + def it_returns_first_match_when_multiple_share_a_name(self, slide_fixture): + # ---add a textbox with a name colliding with a placeholder--- + tb = slide_fixture.shapes.add_textbox(Inches(2), Inches(2), Inches(2), Inches(1)) + tb.name = "Title 1" # ---collide with the existing title's name--- + + # ---first match in document order: the original title placeholder--- + result = slide_fixture.shapes.by_name("Title 1") + # ---identity check: same _element as the placeholder--- + assert result.shape_id == slide_fixture.shapes.title.shape_id + + def it_works_on_slide_layout_shapes(self): + # ---the layout shapes inherit by_name through _BaseShapes--- + prs = Presentation() + layout = prs.slide_layouts[1] + # ---layouts also have a 'Title 1' placeholder--- + title = layout.shapes.by_name("Title 1") + assert title.name == "Title 1" + + def it_works_on_slide_master_shapes(self): + prs = Presentation() + master = prs.slide_master + # ---masters typically have 'Title Placeholder 1'--- + # ---guard against fixture variation: just confirm by_name works on at least one shape--- + names = [s.name for s in master.shapes] + if names: + sh = master.shapes.by_name(names[0]) + assert sh.name == names[0] + + +# --------------------------------------------------------------------------- +# Anti / Regression +# --------------------------------------------------------------------------- + + +class DescribePhase2_Regression(object): + """Anti-criteria — existing surfaces unchanged.""" + + def it_keeps_font_color_setter_working_unchanged(self, run_fixture): + run_fixture.font.color.rgb = RGBColor(0x12, 0x34, 0x56) + assert run_fixture.font.color.rgb == RGBColor(0x12, 0x34, 0x56) + + def it_keeps_phase1_PathLike_working(self, tmp_path): + # ---Phase 1's PathLike fix should still work--- + prs = Presentation() + out = tmp_path / "x.pptx" + prs.save(out) + prs2 = Presentation(out) + assert len(prs2.slides) == 0 + + def it_keeps_phase1_PERCENT_40_typo_fix(self): + from pptx.enum.dml import MSO_PATTERN_TYPE + + # ---Phase 1 fixed ERCENT_40 -> PERCENT_40--- + assert MSO_PATTERN_TYPE.PERCENT_40.value == 6 diff --git a/tests/text/test_text.py b/tests/text/test_text.py index 48bbc5bc1..45455dd26 100644 --- a/tests/text/test_text.py +++ b/tests/text/test_text.py @@ -8,7 +8,6 @@ import pytest -from pptx.dml.color import ColorFormat from pptx.dml.fill import FillFormat from pptx.enum.lang import MSO_LANGUAGE_ID from pptx.enum.text import MSO_ANCHOR, MSO_AUTO_SIZE, MSO_UNDERLINE, PP_ALIGN, PP_BULLET_TYPE @@ -526,7 +525,17 @@ def it_can_change_its_latin_typeface(self, name_set_fixture): assert font._element.xml == expected_xml def it_provides_access_to_its_color(self, font): - assert isinstance(font.color, ColorFormat) + # ---issue #29 Phase 2: Font.color now returns a non-mutating + # ---_LazyFontColorFormat proxy instead of a real ColorFormat, + # ---so reads on an unstyled run don't insert . + # ---The proxy mirrors ColorFormat's public surface + # ---(rgb/theme_color/type/brightness/transparency). + from pptx.text.text import _LazyFontColorFormat + + assert isinstance(font.color, _LazyFontColorFormat) + # ---ColorFormat-shaped duck-typing on the proxy: + for attr in ("rgb", "theme_color", "type", "brightness", "transparency"): + assert hasattr(font.color, attr) def it_provides_access_to_its_fill(self, font): assert isinstance(font.fill, FillFormat)