Skip to content

feat: Font.color non-mutation + UTC-aware datetimes + Shapes.by_name (Modernization Phase 2)#43

Merged
MHoroszowski merged 1 commit intomasterfrom
feature/modernization-phase2
May 8, 2026
Merged

feat: Font.color non-mutation + UTC-aware datetimes + Shapes.by_name (Modernization Phase 2)#43
MHoroszowski merged 1 commit intomasterfrom
feature/modernization-phase2

Conversation

@MHoroszowski
Copy link
Copy Markdown
Owner

Phase 2 of issue #29 (Modernization & Ergonomics): bug fixes + by_name

Phase 1 (PR #39) shipped PathLike + PERCENT_40 typo + Slide.background fix. This PR closes three nagging upstream tickets in a single bundle, all from 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.

Public surface — bug fixes

# Font.color is now non-mutating on read (closes scanny#1111, #1074)
font = run.font
_ = font.color.rgb            # returns None on unstyled run; ZERO XML diff
_ = font.color.type           # returns None on unstyled run; ZERO XML diff
font.color.rgb = RGBColor(...)  # still works — lazy <a:solidFill/> materialization

# core_properties datetimes are tz-aware on read (closes scanny#957)
prs.core_properties.created   # tz-aware datetime when XML carries 'Z' or ±HH:MM
prs.core_properties.created = dt.datetime(2024, 7, 4, 10, 0, 0, tzinfo=PDT)
# ↑ written as UTC: 2024-07-04T17:00:00Z

# Shape lookup helper (closes scanny#798, #309, #532)
slide.shapes.by_name("Title 1")  # → BaseShape  (first match, case-sensitive)
slide.shapes.by_name("Bogus")    # → KeyError

What it adds

Font.color non-mutation (closes scanny#1111, scanny#1074)

  • Font.color returns a new _LazyFontColorFormat proxy that mirrors ColorFormat's public surface.
  • Reads (type, rgb, theme_color, brightness, transparency) return None on a run with no <a:solidFill/> — the inherit signal — without writing to XML.
  • Writes (rgb=, theme_color=, brightness=, transparency=) materialize <a:solidFill/> lazily on first set, then delegate to the real ColorFormat.
  • A pre-commit Forge audit caught my initial draft returning MSO_THEME_COLOR.NOT_THEME_COLOR / 0.0 on the no-fill path — those are real settable values, not inherit signals. Conflating them would silently break inheritance on read/write round-trips. Fixed to return None for all four reads on the no-fill path.

UTC-aware datetimes (closes scanny#957)

  • _parse_W3CDTF_to_datetime returns tz-aware datetimes when the source string carries timezone info: Z → UTC; ±HH:MM → fixed-offset datetime.timezone.
  • Strings without offset markers (year-only, year-month, year-month-day, bare timestamp) continue to return naive datetimes — we don't assume a timezone where none was given.
  • _set_element_datetime accepts both naive and tz-aware datetimes; 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.
  • Sign-convention fix: prior _offset_dt had sign_factor = -1 if "+" else 1 (inverted) and adjusted clock values; new _tzinfo_from_offset_str uses POSIX-correct sign_factor = 1 if "+" else -1 and returns a tzinfo instance. Forge audit walked through '-08:00' and '+05:30' — same effective UTC instant produced in both old and new code (old returned naive-but-UTC-shifted, new returns tz-aware-with-original-offset).

_BaseShapes.by_name(name) (closes scanny#798, scanny#309, scanny#532)

  • Returns the first shape in document order whose .name matches.
  • Case-sensitive (matches PowerPoint's behavior).
  • Raises KeyError with a clear message on miss.
  • Defined on _BaseShapes so it inherits to SlideShapes, SlideLayoutShapes, SlideMasterShapes, etc.

Behavior changes (intentional, called out for downstream consumers)

  • Reading font.color no longer inserts <a:solidFill/>. Code relying on the mutation as a side-effect must call font.fill.solid() explicitly.
  • Reading core_properties.created / last_printed / modified on a document whose XML carries Z or ±HH:MM markers now returns a tz-aware datetime. Tests and callers that expected naive datetimes need updating; this PR updates the two existing test fixtures that exercised this path.

Tests

  • 30 new pytest cases in tests/test_modernization_phase2.py covering byte-stable XML on Font.color reads, lazy materialization on first set, the inherit-signal contract (None for all four reads on no-fill), datetime parser tz-awareness across all five W3CDTF input forms, round-trip through save/reload, and the five 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: 3456 passed.
  • 5 new behave scenarios in features/modernization-phase2.feature. 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 clean: ruff check src tests → All checks passed; ruff format --check → no diff.

Reporting contract (CLAUDE.md §7)

$ python3 -m pytest tests/ -q | tail -3
........................................................................ [ 97%]
........................................................................ [100%]
3456 passed in 4.82s

$ python3 -m ruff check src tests | tail -3
All checks passed!

$ python3 -m behave features/ --no-color 2>&1 | tail -3
1041 scenarios passed, 0 failed, 0 skipped
3134 steps passed, 0 failed, 0 skipped
Took 0min 1.591s

UAT

  • uat_modernization_phase2.py (untracked per CLAUDE.md §6) at repo root.
  • Three demonstrations: byte-diff non-mutation, datetime round-trip with PDT input → UTC reload, by_name hit/miss.
  • All programmatic assertions pass; visual UAT signed off by maintainer.

Skipped from issue #29 Phase 2 (deferred or no-op)

Refs #29

…(Modernization Phase 2)

Issue: #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 `<a:solidFill/>` into the
  run's `<a:rPr>` 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 `<a:solidFill/>` lazily on first write and
  delegates to the real `ColorFormat`. Closes scanny#1111
  and scanny#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#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#798, scanny#309, and
  scanny#532.

Behavior changes (intentional, documented in the PR body)
- Reading `font.color` no longer inserts `<a:solidFill/>`. 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Font.color getter mutates XML by inserting <a:solidFill/>, breaking theme color inheritance

1 participant