Skip to content

[py] Type WebElement.get_attribute name - #18028

Merged
diemol merged 4 commits into
SeleniumHQ:trunkfrom
adamtheturtle:codex/type-get-attribute-name
Sep 21, 2026
Merged

diemol merged 4 commits into
SeleniumHQ:trunkfrom
adamtheturtle:codex/type-get-attribute-name

Conversation

@adamtheturtle

@adamtheturtle adamtheturtle commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

🔗 Related Issues

None.

💥 What does this PR do?

Annotates the attribute or property name accepted by WebElement.get_attribute as str, so strict Pyright consumers no longer see a partially unknown bound method type.

🔧 Implementation Notes

The return type remains str | None; this PR only adds the missing parameter annotation.

Validation:

  • Ruff check and format check passed for the changed file.
  • Python compileall and git diff --check passed.
  • A strict Pyright consumer probe resolves the method as (name: str) -> str | None.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): OpenAI Codex
    • What was generated: Implementation suggestions, type-analysis probes, validation commands, and PR-description wording.
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

This is annotation-only and does not change runtime behavior.

🔄 Types of changes

  • Cleanup (type annotations)

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@selenium-ci selenium-ci added the C-py Python Bindings label Sep 12, 2026
@diemol

diemol commented Sep 14, 2026

Copy link
Copy Markdown
Member

@adamtheturtle can you check the PR description? Looks like the formar went wrong. Also, can you please follow the PR template?

@adamtheturtle

Copy link
Copy Markdown
Contributor Author

Thanks — I fixed the formatting and rewrote the description using the current PR template, including the AI-assistance disclosure.

@qodo-code-review

qodo-code-review Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 58f2a72 🚀 Fast

Results up to commit 0142006 🚀 Fast


No changes from previous review

Grey Divider

Qodo Logo

@diemol diemol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is correct, I don't understand why only adding a type to this method when there are other two that also need this. Ideally we want to have the whole file typed where it applies, and not these one off PRs. I hope that makes sense.

@adamtheturtle

Copy link
Copy Markdown
Contributor Author

@diemol I do not have capacity to expand the scope of this.

Extends this PR's get_attribute annotation to the two sibling
methods that share the same untyped name parameter, as requested
in review.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 58f2a72

@qodo-code-review

qodo-code-review Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

No code changes since the last review — review skipped

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-py Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants