[DanielNg0729]

Summary

• _get_shape_position read the position with getattr(shape, attr). It now types shape as BaseShape and attr as Literal["top", "left"], and reads shape.top or shape.left directly. _iter_shapes_by_position now uses BaseShape instead of object.
• The AttributeError catch in _get_shape_position stays, with a comment. python-pptx wraps an element without a registered class, such as a p:contentPart inside a group, in a plain BaseShape. Its top and left getters then raise AttributeError.
• handle_shapes checked hasattr(shape, "text"). In python-pptx, only autoshape.Shape and its placeholder subclasses define text, so the check is now isinstance(shape, Shape).

The behaviour does not change, so I did not add a test.

Checklist:

• Documentation has been updated, if necessary.
• Examples have been added, if necessary.
• Tests have been added, if necessary.

[github-actions]

✅ DCO Check Passed

Thanks @DanielNg0729, all your commits are properly signed off. 🎉

[mergify]

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

• title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

[github-actions]

AI triage (advisory)

Checked commit a53b2066c1. The analysis read the diff only. It did not run code from this pull request.

This PR refactors the PPTX backend to replace getattr/hasattr attribute probing with explicit typed access: _get_shape_position now takes BaseShape and a Literal["top","left"] parameter, and the hasattr(shape, "text") guard becomes isinstance(shape, Shape). No behaviour change is intended.

Topic labels added: pptx.

CI safety: no concern found. A maintainer must still approve CI.

1 related pull request(s) in the same code

PR	State	Signal	Assessment
#4443	open	changes the same code	related: PR 4443 adds run-level hyperlink and character formatting to the same file and overlaps 3 hunks, but it is a feature addition, not the same refactor. The two PRs cannot both merge without rebasing.

_{The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

AI first review (advisory): no blocking issue found

This review covers the full diff at a53b2066c1. It read the code only. It did not run tests or code from this pull request.

The PR replaces getattr(shape, attr) in _get_shape_position with direct shape.top / shape.left access. It types the shape helpers with BaseShape and Literal["top", "left"], and it replaces hasattr(shape, "text") in handle_shapes with isinstance(shape, Shape). I found no wrong behavior. from __future__ import annotations keeps BaseShape out of runtime evaluation, so the guarded pptx import stays safe. Shape is used at runtime only after the constructor confirms that pptx is available. Limits: I did not run the tests or ty. I could not confirm from the workspace that text exists only on autoshape.Shape and its placeholder subclasses, because the python-pptx source is not installed here. The main remaining risk is a merge conflict with open PR #4443, which changes the same hunks.

_{A maintainer makes the merge decision. This review never approves the pull request.}

[PeterStaar-IBM]

lgtm!