Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[py] correct type annotations of default-None params #15341

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

DeflateAwning
Copy link

@DeflateAwning DeflateAwning commented Feb 25, 2025

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.
  • @CLAassistant
    Copy link

    CLAassistant commented Feb 25, 2025

    CLA assistant check
    All committers have signed the CLA.

    Copy link
    Contributor

    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.

    Copy link
    Contributor

    qodo-merge-pro bot commented Feb 25, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    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

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

    Successfully merging this pull request may close these issues.

    2 participants