andpp/mopidy
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge pull request #861 from jodal/fix/837-model-eq

Browse Source 
Cleanup model repr() and fix equality of models with fields set to default values
This commit is contained in:
Thomas Adamcik 2014-09-22 22:32:49 +02:00
commit ac777c17d6
4 changed files with 51 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
docs/changelog.rst
View File

@ -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)
====================

22
mopidy/models.py
View File

@ -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 [])

6
mopidy/mpd/translator.py
View File

@ -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))

37
tests/test_models.py
View File

@ -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):