Skip to content

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

Merged
cgoldberg merged 3 commits into
SeleniumHQ:trunkfrom
saiyam-jain-dev:fix-py-script-key-hash
Oct 27, 2026
Merged

cgoldberg merged 3 commits into
SeleniumHQ:trunkfrom
saiyam-jain-dev:fix-py-script-key-hash

Conversation

@saiyam-jain-dev

@saiyam-jain-dev saiyam-jain-dev commented Oct 26, 2026 •

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
@cgoldberg
cgoldberg self-requested a review October 27, 2026 19:41

@cgoldberg cgoldberg 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.

LGTM 👍 thanks for keeping the PRs small and focused!

@cgoldberg
cgoldberg merged commit ee6af75 into SeleniumHQ:trunk Oct 27, 2026
32 checks passed
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