Skip to content

[py] Implement __hash__ and explicit ScriptKey equality comparison - #18243

Open
saiyam-jain-dev wants to merge 1 commit into
SeleniumHQ:trunkfrom
saiyam-jain-dev:fix-py-script-key-hash
Open

saiyam-jain-dev wants to merge 1 commit into
SeleniumHQ:trunkfrom
saiyam-jain-dev:fix-py-script-key-hash

Conversation

@saiyam-jain-dev

Copy link
Copy Markdown
Contributor

Description

\ScriptKey\ is used in the Python bindings as an identifier for pinned scripts (\driver.pin_script(), \driver.unpin()).

Previously:

  1. \ScriptKey\ defined _eq_\ without defining _hash_, which in Python 3 implicitly sets _hash_ = None. Attempting to use a \ScriptKey\ in a dictionary or set raised \TypeError: unhashable type: 'ScriptKey'.
  2. _eq_\ compared \self._id == other\ directly without checking whether \other\ is an instance of \ScriptKey\ or a raw ID, relying on reflected equality for \ScriptKey == ScriptKey.

This PR:

  • Implements _hash_\ returning \hash(self._id)\ so \ScriptKey\ instances can be hashed and stored in sets or mappings.
  • Compares \self._id == other._id\ when \other\ is an instance of \ScriptKey, while preserving raw ID comparisons.
  • Adds comprehensive unit tests in \py/test/unit/selenium/webdriver/remote/script_key_tests.py.

Motivation and Context

Enables using \ScriptKey\ objects as keys in hash mappings/sets and resolves Ruff warning \PLW1641.

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.

@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 Oct 26, 2026

This branch has not been deployed

No deployments
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.

2 participants