Skip to content

Remove PyQualifier and ConstPyQualifier classes#304

Open
tomaskontrimas wants to merge 1 commit intomasterfrom
remove_PyQualifier
Open

Remove PyQualifier and ConstPyQualifier classes#304
tomaskontrimas wants to merge 1 commit intomasterfrom
remove_PyQualifier

Conversation

@tomaskontrimas
Copy link
Copy Markdown
Collaborator

Introduced in 8a64418 but never used in the package. Removing them as they would make typing and inheritance inspection even more difficult.

@tomaskontrimas tomaskontrimas self-assigned this Apr 19, 2026
Copilot AI review requested due to automatic review settings April 19, 2026 14:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the unused PyQualifier / ConstPyQualifier mechanism from skyllh.core.py, simplifying runtime typing/inspection utilities and aligning the test suite with the reduced API surface.

Changes:

  • Removed PyQualifier, ConstPyQualifier, and the const instance from skyllh/core/py.py.
  • Simplified issequenceof by dropping qualifier-filtering support and reducing it to a pure isinstance sequence check.
  • Updated tests/core/test_py.py by removing qualifier-related tests and imports.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
skyllh/core/py.py Deletes qualifier classes and simplifies issequenceof implementation/signature accordingly.
tests/core/test_py.py Removes tests/imports that exercised the deleted qualifier behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skyllh/core/py.py
Comment on lines +164 to +166
def issequenceof(obj, T):
"""Checks if the given object ``obj`` is a sequence with items being
instances of type ``T``, possibly qualified with the given Python
qualifiers.
instances of type ``T``.
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

Removing the optional pyqualifiers parameter from issequenceof is a backwards-incompatible public API change (callers passing the 3rd argument will now get a TypeError). If this function is part of the supported API, consider keeping the parameter (e.g., pyqualifiers=None or *pyqualifiers) for compatibility and either ignoring it or raising a clear error when non-None is provided, with a deprecation path if desired.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

no

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants