Skip to content
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

ID3: Incorrect __eq__ method in TextFrame class #43

lazka opened this issue Jul 4, 2014 · 1 comment

ID3: Incorrect __eq__ method in TextFrame class #43

lazka opened this issue Jul 4, 2014 · 1 comment


Copy link

@lazka lazka commented Jul 4, 2014

Originally reported by: Christoph Reiter (Bitbucket: lazka, GitHub: lazka)

From on December 19, 2009 13:48:50

This is not really causing me any problems, just found this 
while messing with the code.

TextFrame class defines both __str__ and __unicode__ methods.
When compared for equality (using __eq__(self, other) method):
In case other is string:
  It converts itself with str(self) for comparison
In case other is unicode:
  It creates a unicode literal u'\u0000'.join(self.text)
  and uses that for comparison. This identical to what the __unicode__
  method returns and at this point is just a slight code duplication.

But there's trouble when a subclass redefines __unicode__,
which is what TimeStampTextFrame does.

What's even more, because TimeStampTextFrame's text attribute is a list
of ID3TimeStamp objects, it actually raises an exception because join()
expects a list of string or unicode literals.

In effect, comparing an instance of TimeStampTextFrame class or it's
subclasses to a unicode literal raises a TypeError, while comparing
to a string does not.

I've attached an obvious solution of just using unicode(self). Don't
really know how to go about tests, so just added a simple TestCase
subclass, also included in the diff. Should I've incorporated it into a
previous test (perhaps somewhere in TestReadTags?), or maybe was it not
necessary at all?

Attachment: TextFrame_eq_patch_and_test.diff

Original issue:


This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):

From on December 19, 2009 14:06:33

This issue was closed by revision r68 .

Status: Fixed
Mergedinto: -

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
1 participant
You can’t perform that action at this time.