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.CRM is useless and buggy #239

Closed
lazka opened this issue Nov 7, 2015 · 4 comments
Closed

id3.CRM is useless and buggy #239

lazka opened this issue Nov 7, 2015 · 4 comments
Labels
bug

Comments

@lazka
Copy link
Member

@lazka lazka commented Nov 7, 2015

Originally reported by: Daniel Plachotich (Bitbucket: danpla, GitHub: danpla)


When certain 2.2 tag is added to ID3, the add() method converts it to a 2.3/4 one by using name of the parent class. The problem is that CRM tag is derived directly from Frame and has no 2.3 analog.

Adding CRM tag (manually or by reading from a file) results the following:

#!python

>>> t = id3.ID3()
>>> t.add(id3.CRM())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.4/dist-packages/mutagen/id3/__init__.py", line 353, in add
    return self.loaded_frame(frame)
  File "/usr/local/lib/python3.4/dist-packages/mutagen/id3/__init__.py", line 345, in loaded_frame
    tag = type(tag).__base__(tag)
  File "/usr/local/lib/python3.4/dist-packages/mutagen/id3/_frames.py", line 62, in __init__
    other._to_other(self)
  File "/usr/local/lib/python3.4/dist-packages/mutagen/id3/_frames.py", line 77, in _to_other
    raise ValueError
ValueError
>>> 

Because:

#!python

>>> id3.Frame()._framespec is not id3.CRM()._framespec
True

This means that no one uses CRM and it can be simply removed from Mutagen to avoid confusion. ID3.add(), however, must be fixed to ignore this tag.


@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Nov 11, 2015

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


We should error out when trying to add such a frame and silently ignore it when loading from a file.

Removing it works as well.. but it's an API break which I'd try to avoid first.

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Nov 11, 2015

Original comment by Daniel Plachotich (Bitbucket: danpla, GitHub: danpla):


But due the bug, there is a 100% guarantee that it has never been used. Maybe it is better to avoid confusion for future users than leaving traces of the feature that has never worked?

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Nov 22, 2015

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


One could still import CRM from id3 which would then fail so that isn't 100%. But we could of course ignore that case.

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Nov 24, 2015

Original comment by Daniel Plachotich (Bitbucket: danpla, GitHub: danpla):


Ideally, all v2.2 frames had to be made as implementation details, since mutagen doesn't write v2.2 tags and just silently converts them to v2.3/4.

@lazka lazka added critical bug labels Apr 7, 2016
@lazka lazka removed the critical label Apr 27, 2016
@lazka lazka closed this in 1324084 Sep 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.