[py] correct type annotations of default-None params by DeflateAwning · Pull Request #15341 · SeleniumHQ/selenium

DeflateAwning · 2025-02-25T21:31:43Z

User description

Corrected type hints/annotations, mostly of arguments with default values of None.

Also added a few entirely-missing type hints, mostly for strings.

Motivation and Context

Pyright type checker was throwing errors in many of these cases.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

I have read the contributing document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

None of the other checklist items are required.

Recommend a squash merge.

PR Type

Bug fix, Enhancement

Description

Corrected type annotations for parameters with default None values.
Added missing type hints, primarily for strings and lists.
Improved type safety by using Optional and explicit type declarations.
Enhanced code clarity and compatibility with type checkers like Pyright.

Changes walkthrough 📝

Relevant files

Bug fix

18 files

service.py Updated type hints for `log_output` parameter.	+1/-1
webdriver.py Added `Optional` type hints for `options` and `service`.	+4/-2
service.py `Corrected type hints for executable_path, log_output, and driver_path_env_key.`	+3/-3
webdriver.py Added `Optional` type hints for multiple parameters.	+5/-3
wheel_actions.py Updated type hint for `source` parameter.	+4/-1
service.py Corrected type hints for multiple parameters including `log_output`.	+3/-3
service.py `Updated type hints for executable_path, log_output, and driver_path_env_key.`	+3/-3
webdriver.py Added `Optional` type hints for `options` and `service`.	+4/-2
service.py `Corrected type hints for executable_path, log_output, and driver_path_env_key.`	+3/-3
webdriver.py Added `Optional` type hints for `options` and `service`.	+3/-2
service.py `Updated type hints for executable_path, log_output, and driver_path_env_key.`	+3/-3
webdriver.py Added `Optional` type hints for `options` and `service`.	+4/-2
remote_connection.py Added `Optional` type hint for `_client_config`.	+1/-1
service.py Corrected type hints for `executable_path` and `driver_path_env_key`.	+2/-2
webdriver.py Added `Optional` type hints for `options` and `service`.	+4/-2
event_firing_webdriver.py Updated type hint for `execute_script` method.	+1/-1
webdriver.py Added `Optional` type hint for `service` parameter.	+2/-1
webdriver.py Added `Optional` type hint for `service` parameter.	+2/-1

Enhancement

2 files

options.py Added type hints for `arguments` and `add_argument` methods.	+3/-2
webdriver.py Enhanced type hints for `execute` and `execute_script` methods.	+3/-2

Need help?
Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
Check out the documentation for more information.

Used regex for search: : \w+ = None

CLAassistant · 2025-02-25T21:31:49Z

All committers have signed the CLA.

qodo-code-review · 2025-02-25T21:32:31Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Type Safety The execute() method's params parameter type was changed from 'dict' to 'dict[str, Any]'. Verify this doesn't break existing code that passes dictionaries with non-string keys. def execute(self, driver_command: str, params: dict[str, Any]) -> dict[str, Any]: """Sends a command to be executed by a command.CommandExecutor.

qodo-code-review · 2025-02-25T21:33:02Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Learned best practice	✅ ~~Use explicit None checks instead of implicit truthiness checks when validating nullable parameters~~ Suggestion Impact: Changed the source parameter check from 'if not source' to 'if source is None' for more explicit null checking code diff: - if not source: + if source is None: Add a null validation check for the `source` parameter at the start of the `init` method to make the code more defensive and prevent potential NullReferenceExceptions. The current code implicitly handles None but an explicit check would be clearer. py/selenium/webdriver/common/actions/wheel_actions.py [25-28] def __init__(self, source: Optional[WheelInput] = None): - if not source: + if source is None: source = WheelInput("wheel") super().__init__(source) `[Suggestion has been applied]` Suggestion importance[1-10]: 6	Low
Update

cgoldberg · 2025-05-13T13:32:00Z

Is this still relevant?

DeflateAwning · 2025-05-13T18:11:55Z

Yes of coures it is still relevant. It's type hints. Not sure why it's taken so long to deal with

cgoldberg · 2025-05-13T20:22:24Z

Not sure why it's taken so long to deal with

It's a community project run by volunteers who put in a lot of effort and get paid nothing. We are pretty diligent with reviewing contributions, but not everything gets immediate attention.

DeflateAwning · 2025-05-13T21:23:18Z

Let me know if I can help expedite things! There's no logic changes or anything, so it shouldn't take much to review

cgoldberg · 2025-05-13T21:43:07Z

@DeflateAwning,

This is causing test failures.

I can reproduce when using your branch:

>>> from selenium import webdriver
>>> driver = webdriver.Chrome()
>>> print(driver.title)
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    print(driver.title)
          ^^^^^^^^^^^^
  File "/home/cgoldberg617/code/selenium/py/selenium/webdriver/remote/webdriver.py", line 487, in title
    return self.execute(Command.GET_TITLE).get("value", "")
           ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^
TypeError: WebDriver.execute() missing 1 required positional argument: 'params'

Can you take a look?

cgoldberg

fix TypeError

DeflateAwning · 2025-05-13T22:34:30Z

My bad, tiny mistake in my haste. Fixed! Can you please re-run CI?

cgoldberg · 2025-05-14T00:01:51Z

CI failed, but it wasn't related to these changes... We have a flaky test. I'm running it again.

cgoldberg

@DeflateAwning
CI is green and this looks good to go.

Thanks for the contribution!

DeflateAwning · 2025-05-14T00:18:12Z

Thanks a lot! Really appreciate it!

DeflateAwning added 2 commits February 25, 2025 14:19

Fix specific type hints

fa11762

Fix type hints with default nulls - all remaining

e2a127e

Used regex for search: : \w+ = None

qodo-code-review bot added the Review effort 2/5 label Feb 25, 2025

DeflateAwning and others added 4 commits February 25, 2025 14:34

Fix executable_path type hint

a4054c7

Implement AI code review suggestion

b28edd9

Merge branch 'trunk' into fix-type-hints

da07f96

Merge branch 'trunk' into fix-type-hints

56594d0

VietND96 requested review from AutomatedTester, cgoldberg and titusfortner March 4, 2025 02:59

Merge branch 'trunk' into fix-type-hints

d42e1b4

cgoldberg requested changes May 13, 2025

View reviewed changes

fix: Re-add default argument I accidentally removed

73f0af9

cgoldberg approved these changes May 14, 2025

View reviewed changes

cgoldberg merged commit ff55efd into SeleniumHQ:trunk May 14, 2025
18 of 19 checks passed

DeflateAwning deleted the fix-type-hints branch May 14, 2025 00:18

This was referenced Sep 12, 2025

Bump the functionaltests group with 8 updates israurreacarballo/DORA#196

Closed

Bump coverlet.collector and 5 others israurreacarballo/DORA#201

Closed

Bump the selenium group with 2 updates 7474/SRC#677

Merged

This was referenced Sep 19, 2025

Bump coverlet.collector and 5 others israurreacarballo/DORA#217

Closed

Bump the functionaltests group with 9 updates israurreacarballo/DORA#225

Closed

Bump coverlet.collector and 5 others israurreacarballo/DORA#228

Closed

dependabot bot mentioned this pull request Oct 24, 2025

Bump Selenium.WebDriver from 4.7.0 to 4.37.0 javiertuya/selema#945

Closed

Uh oh!

Conversation

DeflateAwning commented Feb 25, 2025 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Motivation and Context

Types of changes

Checklist

PR Type

Description

Changes walkthrough 📝

Uh oh!

CLAassistant commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-code-review bot commented Feb 25, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review bot commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

cgoldberg commented May 13, 2025

Uh oh!

DeflateAwning commented May 13, 2025

Uh oh!

cgoldberg commented May 13, 2025

Uh oh!

DeflateAwning commented May 13, 2025

Uh oh!

cgoldberg commented May 13, 2025

Uh oh!

cgoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

DeflateAwning commented May 13, 2025

Uh oh!

cgoldberg commented May 14, 2025

Uh oh!

cgoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DeflateAwning commented May 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DeflateAwning commented Feb 25, 2025 •

edited by qodo-code-review bot

Loading

CLAassistant commented Feb 25, 2025 •

edited

Loading

qodo-code-review bot commented Feb 25, 2025 •

edited

Loading