diff --git a/docs/changelog.rst b/docs/changelog.rst index 0ff362f8..f50ff426 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -41,6 +41,20 @@ v0.20.0 (UNRELEASED) make sense for a server such as Mopidy. Currently the only way to find out if it is in use and will be missed is to go ahead and remove it. +**Models** + +- Hide empty collections from :func:`repr()` representations. + +- Field values are no longer stored on the model instance when the value + matches the default value for the field. This makes two models equal when + they have a field which in one case is implicitly set to the default value + and in the other case explicitly set to the default value, but with otherwise + equal fields. (Fixes: :issue:`837`) + +- Changed the default value of :attr:`mopidy.models.Album.num_tracks`, + :attr:`mopidy.models.Track.track_no`, and + :attr:`mopidy.models.Track.last_modified` from ``0`` to :class:`None`. + v0.19.4 (2014-09-01) ==================== diff --git a/mopidy/models.py b/mopidy/models.py index 42313922..bedf8ca5 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -18,6 +18,8 @@ class ImmutableObject(object): raise TypeError( '__init__() got an unexpected keyword argument "%s"' % key) + if value == getattr(self, key): + continue # Don't explicitly set default values self.__dict__[key] = value def __setattr__(self, name, value): @@ -29,6 +31,8 @@ class ImmutableObject(object): kwarg_pairs = [] for (key, value) in sorted(self.__dict__.items()): if isinstance(value, (frozenset, tuple)): + if not value: + continue value = list(value) kwarg_pairs.append('%s=%s' % (key, repr(value))) return '%(classname)s(%(kwargs)s)' % { @@ -70,13 +74,11 @@ class ImmutableObject(object): for key in self.__dict__.keys(): public_key = key.lstrip('_') value = values.pop(public_key, self.__dict__[key]) - if value is not None: - data[public_key] = value + data[public_key] = value for key in values.keys(): if hasattr(self, key): value = values.pop(key) - if value is not None: - data[key] = value + data[key] = value if values: raise TypeError( 'copy() got an unexpected keyword argument "%s"' % key) @@ -239,7 +241,7 @@ class Album(ImmutableObject): :param artists: album artists :type artists: list of :class:`Artist` :param num_tracks: number of tracks in album - :type num_tracks: integer + :type num_tracks: integer or :class:`None` if unknown :param num_discs: number of discs in album :type num_discs: integer or :class:`None` if unknown :param date: album release date (YYYY or YYYY-MM-DD) @@ -260,7 +262,7 @@ class Album(ImmutableObject): artists = frozenset() #: The number of tracks in the album. Read-only. - num_tracks = 0 + num_tracks = None #: The number of discs in the album. Read-only. num_discs = None @@ -300,7 +302,7 @@ class Track(ImmutableObject): :param genre: track genre :type genre: string :param track_no: track number in album - :type track_no: integer + :type track_no: integer or :class:`None` if unknown :param disc_no: disc number in album :type disc_no: integer or :class:`None` if unknown :param date: track release date (YYYY or YYYY-MM-DD) @@ -314,7 +316,7 @@ class Track(ImmutableObject): :param musicbrainz_id: MusicBrainz ID :type musicbrainz_id: string :param last_modified: Represents last modification time - :type last_modified: integer + :type last_modified: integer or :class:`None` if unknown """ #: The track URI. Read-only. @@ -339,7 +341,7 @@ class Track(ImmutableObject): genre = None #: The track number in the album. Read-only. - track_no = 0 + track_no = None #: The disc number in the album. Read-only. disc_no = None @@ -362,7 +364,7 @@ class Track(ImmutableObject): #: Integer representing when the track was last modified, exact meaning #: depends on source of track. For local files this is the mtime, for other #: backends it could be a timestamp or simply a version counter. - last_modified = 0 + last_modified = None def __init__(self, *args, **kwargs): get = lambda key: frozenset(kwargs.pop(key, None) or []) diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py index 252725ee..f3264a46 100644 --- a/mopidy/mpd/translator.py +++ b/mopidy/mpd/translator.py @@ -44,11 +44,11 @@ def track_to_mpd_format(track, position=None): if track.date: result.append(('Date', track.date)) - if track.album is not None and track.album.num_tracks != 0: + if track.album is not None and track.album.num_tracks is not None: result.append(('Track', '%d/%d' % ( - track.track_no, track.album.num_tracks))) + track.track_no or 0, track.album.num_tracks))) else: - result.append(('Track', track.track_no)) + result.append(('Track', track.track_no or 0)) if position is not None and tlid is not None: result.append(('Pos', position)) result.append(('Id', tlid)) diff --git a/tests/test_models.py b/tests/test_models.py index 43343ce7..56d6c76b 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -171,8 +171,8 @@ class ArtistTest(unittest.TestCase): def test_serialize_falsy_values(self): self.assertDictEqual( - {'__model__': 'Artist', 'uri': '', 'name': None}, - Artist(uri='', name=None).serialize()) + {'__model__': 'Artist', 'uri': '', 'name': ''}, + Artist(uri='', name='').serialize()) def test_to_json_and_back(self): artist1 = Artist(uri='uri', name='name') @@ -318,13 +318,12 @@ class AlbumTest(unittest.TestCase): def test_repr_without_artists(self): self.assertEquals( - "Album(artists=[], images=[], name=u'name', uri=u'uri')", + "Album(name=u'name', uri=u'uri')", repr(Album(uri='uri', name='name'))) def test_repr_with_artists(self): self.assertEquals( - "Album(artists=[Artist(name=u'foo')], images=[], name=u'name', " - "uri=u'uri')", + "Album(artists=[Artist(name=u'foo')], name=u'name', uri=u'uri')", repr(Album(uri='uri', name='name', artists=[Artist(name='foo')]))) def test_serialize_without_artists(self): @@ -551,14 +550,12 @@ class TrackTest(unittest.TestCase): def test_repr_without_artists(self): self.assertEquals( - "Track(artists=[], composers=[], name=u'name', " - "performers=[], uri=u'uri')", + "Track(name=u'name', uri=u'uri')", repr(Track(uri='uri', name='name'))) def test_repr_with_artists(self): self.assertEquals( - "Track(artists=[Artist(name=u'foo')], composers=[], name=u'name', " - "performers=[], uri=u'uri')", + "Track(artists=[Artist(name=u'foo')], name=u'name', uri=u'uri')", repr(Track(uri='uri', name='name', artists=[Artist(name='foo')]))) def test_serialize_without_artists(self): @@ -738,6 +735,18 @@ class TrackTest(unittest.TestCase): self.assertNotEqual(track1, track2) self.assertNotEqual(hash(track1), hash(track2)) + def test_ignores_values_with_default_value_none(self): + track1 = Track(name='name1') + track2 = Track(name='name1', album=None) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + + def test_copy_can_reset_to_default_value(self): + track1 = Track(name='name1') + track2 = Track(name='name1', album=Album()).copy(album=None) + self.assertEqual(track1, track2) + self.assertEqual(hash(track1), hash(track2)) + class TlTrackTest(unittest.TestCase): def test_tlid(self): @@ -773,8 +782,7 @@ class TlTrackTest(unittest.TestCase): def test_repr(self): self.assertEquals( - "TlTrack(tlid=123, track=Track(artists=[], composers=[], " - "performers=[], uri=u'uri'))", + "TlTrack(tlid=123, track=Track(uri=u'uri'))", repr(TlTrack(tlid=123, track=Track(uri='uri')))) def test_serialize(self): @@ -903,13 +911,12 @@ class PlaylistTest(unittest.TestCase): def test_repr_without_tracks(self): self.assertEquals( - "Playlist(name=u'name', tracks=[], uri=u'uri')", + "Playlist(name=u'name', uri=u'uri')", repr(Playlist(uri='uri', name='name'))) def test_repr_with_tracks(self): self.assertEquals( - "Playlist(name=u'name', tracks=[Track(artists=[], composers=[], " - "name=u'foo', performers=[])], uri=u'uri')", + "Playlist(name=u'name', tracks=[Track(name=u'foo')], uri=u'uri')", repr(Playlist(uri='uri', name='name', tracks=[Track(name='foo')]))) def test_serialize_without_tracks(self): @@ -1036,7 +1043,7 @@ class SearchResultTest(unittest.TestCase): def test_repr_without_results(self): self.assertEquals( - "SearchResult(albums=[], artists=[], tracks=[], uri=u'uri')", + "SearchResult(uri=u'uri')", repr(SearchResult(uri='uri'))) def test_serialize_without_results(self):