Skip to content

Conversation

pallavigitwork
Copy link
Member

@pallavigitwork pallavigitwork commented Sep 2, 2025

User description

🔗 Related Issues

Fixes the mypy error from #15697 - interaction file and related files

💥 What does this PR do?

the mypy listed out error for the wheel action python file and other related file. it is because of type mismatch.
changed interaction to expect InputDevice as source passed, instead of str data type.
changed other files related to this.

🔧 Implementation Notes

💡 Additional Considerations

ran the format script
and the pytest .

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)

PR Type

Bug fix


Description

  • Fixed mypy type error in Interaction and other related files.

  • modified the interaction to expect source as InputDevice and not string.


Diagram Walkthrough

flowchart LR
  A["WheelActions.__init__"] --> B["super().__init__(source.type)"]
  A --> C["self._source = source"]
  D["pause() method"] --> E["self._source.create_pause()"]
  F["scroll() method"] --> G["self._source.create_scroll()"]
Loading

File Walkthrough

Relevant files
Bug fix
wheel_actions.py
Fix mypy type error in WheelActions                                           

py/selenium/webdriver/common/actions/wheel_actions.py

  • Modified constructor to pass source.type to parent class instead of
    source object
  • Added self._source attribute to store the wheel input source
  • Updated pause() and scroll() methods to use self._source instead of
    self.source
+4/-3     

@selenium-ci selenium-ci added the C-py Python Bindings label Sep 2, 2025
Copy link
Contributor

qodo-merge-pro bot commented Sep 2, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5678 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Diagnose and resolve "Error: ConnectFailure (Connection refused)" occurring when instantiating additional ChromeDriver instances.
  • Ensure reliability when creating multiple ChromeDriver instances on Ubuntu 16.04 with Chrome 65 and ChromeDriver 2.35.
  • Provide a fix or guidance that prevents connection failures after the first instance.

Requires further human verification:

  • Reproduce multi-instance ChromeDriver behavior across supported environments to validate any potential fixes.

1234 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Clicking a link with JavaScript in href should trigger JavaScript (as in 2.47.1) for Firefox around version 42.
  • Identify regression between 2.47.1 and 2.48.x for click() behavior.
  • Provide code changes or test coverage to restore expected behavior.

Requires further human verification:

  • Browser-level manual verification on affected Firefox versions.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

API Consistency

Switching from self.source to self._source may affect external code if source was part of the public interface; confirm no public attribute contract is broken and that Interaction no longer relies on the original source object.

    super().__init__(source.type)
    self._source = source

def pause(self, duration: float = 0):
    self._source.create_pause(duration)
    return self

def scroll(self, x=0, y=0, delta_x=0, delta_y=0, duration=0, origin="viewport"):
    self._source.create_scroll(x, y, delta_x, delta_y, duration, origin)
    return self
Type Safety

Passing source.type (a string) to Interaction.__init__ assumes the base class expects a string. Verify all Interaction methods used downstream do not require the full WheelInput object.

super().__init__(source.type)
self._source = source

Copy link
Contributor

qodo-merge-pro bot commented Sep 2, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@navin772
Copy link
Member

navin772 commented Sep 2, 2025

The changes look good, could you also fix mypy errors in pointer_actions.py, it's quite similar and relevant to add in this PR.

@pallavigitwork
Copy link
Member Author

I am under assumption we keep PRs small . I'm going by fixing mypy errors per file I see them which aren't fix yet.
If it's okay this fix to be merged please kindly merge it for this file.

@cgoldberg cgoldberg changed the title fixed the mypy error for the wheel action python file [py] Fix mypy error in wheel actions Sep 2, 2025
@cgoldberg
Copy link
Contributor

That's really confusing that the base Interaction class requires a string for source, but subclasses use a source object?

Instead of the changes in this PR, would it make sense to change the type annotation on the Interaction.__init__ method to not require a string? This seems like we are making changes to satisfy the type checker for an incorrect type annotation.

@pallavigitwork
Copy link
Member Author

I have another question to this, before mypy errors were generated how did this code previously function? I noticed what you mentioned. Interaction is a parent class to more than one child classes. I am asking to understand how did this work before mypy errors? Sorry if this question sounds basic for python I'm trying to make sense of this.

@cgoldberg
Copy link
Contributor

how did this work before mypy errors?

In the WheelActions class, source is an instance of WheelInput, which is a subclass of
InputDevice. I think in the base Interaction class, the type annotation for source should be InputDevice, not string.

The type hint in the base class looks incorrect to me, but it works fine because type hints aren't enforced at runtime.. they just cause errors in mypy.

@pallavigitwork
Copy link
Member Author

pallavigitwork commented Sep 2, 2025

Thanks for the explanation Corey.

Made changes, kindly review
@navin772 / @cgoldberg

@pallavigitwork pallavigitwork changed the title [py] Fix mypy error in wheel actions [py] Fix mypy errors in file Interaction and other related classes Sep 3, 2025
@pallavigitwork pallavigitwork changed the title [py] Fix mypy errors in file Interaction and other related classes [py] Fix mypy errors in file Interaction and other related files Sep 3, 2025
@cgoldberg cgoldberg changed the title [py] Fix mypy errors in file Interaction and other related files [py] Fix mypy errors Sep 3, 2025
@pallavigitwork pallavigitwork requested review from a team and cgoldberg and removed request for a team September 4, 2025 06:22
@pallavigitwork
Copy link
Member Author

@cgoldberg , requesting review and merge of the PR. if any more changes aren't required.

Copy link
Contributor

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

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

@pallavigitwork sorry for the delay. I am requesting one tiny change.. besides that it LGTM. I'll merge it as soon as you make the change from my comment.

Copy link
Contributor

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@cgoldberg cgoldberg merged commit 334b760 into SeleniumHQ:trunk Sep 6, 2025
16 checks passed
@pallavigitwork
Copy link
Member Author

Thank you .

@pallavigitwork pallavigitwork deleted the 15697Fixx branch September 8, 2025 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants