Skip to content

Fix repr of Signature so it works for subclasses #1552

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

Closed
wants to merge 1 commit into from

Conversation

robtaylor
Copy link

At the moment the __repr__ for Signature doesn't work for derived classes of Signature (as used e.g. in Glasgow).

Simple fix here.

@robtaylor robtaylor requested a review from whitequark as a code owner January 18, 2025 09:44
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

Apologies, it seems like I've never submitted a review.

@@ -1002,7 +1002,7 @@ def annotations(self, obj, /):
return tuple()

def __repr__(self):
if type(self) is Signature:
if isinstance(self, Signature):
return f"Signature({dict(self.members.items())})"
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a correct fix. There are two problems with it.

  1. A repr is supposed to give information about the class name as well. Your fix would incorrectly indicate that the class name is Signature rather than whatever the subclass is.
  2. Subclassing Signature is only really useful if you're going to add non-member attributes to it. But the default implementation only prints members. The check here serves as a nudge to implementers to make sure they really did cover all attributes.

@robtaylor
Copy link
Author

robtaylor commented Jul 4, 2025 via email

@whitequark whitequark closed this Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants