Skip to content

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.

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