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

models: Deprecate copy and add replace method

Browse Source 
Changed as with the memoization copy was lying, so replace is a better name.
This commit is contained in:
Thomas Adamcik 2015-04-08 01:41:50 +02:00
parent b7375323e9
commit 05244f7e60
4 changed files with 65 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
docs/api/models.rst
View File

@ -8,7 +8,7 @@ and immutable. In other words, they can only be set through the class
constructor during instance creation.
If you want to modify a model, use the
:meth:`~mopidy.models.ImmutableObject.copy` method. It accepts keyword
:meth:`~mopidy.models.ImmutableObject.replace` method. It accepts keyword
arguments for the parts of the model you want to change, and copies the rest of
the data from the model you call it on. Example::
@ -16,7 +16,7 @@ the data from the model you call it on. Example::
>>> track1 = Track(name='Christmas Carol', length=171)
>>> track1
Track(artists=[], length=171, name='Christmas Carol')
>>> track2 = track1.copy(length=37)
>>> track2 = track1.replace(length=37)
>>> track2
Track(artists=[], length=37, name='Christmas Carol')
>>> track1
@ -75,6 +75,7 @@ Data model helpers
==================
.. autoclass:: mopidy.models.ImmutableObject
:members:
.. autoclass:: mopidy.models.Field

34
mopidy/models.py
View File

@ -4,6 +4,8 @@ import copy
import json
import weakref
from mopidy.utils import deprecation
# TODO: split into base models, serialization and fields?
@ -166,6 +168,10 @@ class ImmutableObject(object):
constructor. Fields should be :class:`Field` instances to ensure type
safety in our models.
Note that since these models can not be changed, we heavily memoize them
to save memory. So constructing a class with the same arguments twice will
give you the same instance twice.
:param kwargs: kwargs to set as fields on the object
:type kwargs: any
"""
@ -224,23 +230,35 @@ class ImmutableObject(object):
def copy(self, **values):
"""
Copy the model with ``field`` updated to new value.
.. deprecated:: 1.1
Use :meth:`replace` instead. Note that we no longer return copies.
"""
deprecation.warn('model.immutable.copy')
return self.replace(**values)
def replace(self, **kwargs):
"""
Replace the fields in the model and return a new instance
Examples::
# Returns a track with a new name
Track(name='foo').copy(name='bar')
Track(name='foo').replace(name='bar')
# Return an album with a new number of tracks
Album(num_tracks=2).copy(num_tracks=5)
Album(num_tracks=2).replace(num_tracks=5)
:param values: the model fields to modify
:type values: dict
:rtype: new instance of the model being copied
Note that internally we memoize heavily to keep memory usage down given
our overly repetitive data structures. So you might get an existing
instance if it contains the same values.
:param kwargs: kwargs to set as fields on the object
:type kwargs: any
:rtype: instance of the model with replaced fields
"""
if not values:
if not kwargs:
return self
other = copy.copy(self)
for key, value in values.items():
for key, value in kwargs.items():
if key not in self._fields:
raise TypeError(
'copy() got an unexpected keyword argument "%s"' % key)

3
mopidy/utils/deprecation.py
View File

@ -40,6 +40,9 @@ _MESSAGES = {
'tracklist.add() "tracks" argument is deprecated',
'core.tracklist.add:uri_arg':
'tracklist.add() "uri" argument is deprecated',
'models.immutable.copy':
'ImmutableObject.copy() is deprecated, use ImmutableObject.replace()',
}

66
tests/models/test_models.py
View File

@ -19,58 +19,58 @@ class CachingTest(unittest.TestCase):
def test_different_instance_with_different_values(self):
self.assertIsNot(Track(uri='test1'), Track(uri='test2'))
def test_different_instance_with_copy(self):
def test_different_instance_with_replace(self):
t = Track(uri='test1')
self.assertIsNot(t, t.copy(uri='test2'))
self.assertIsNot(t, t.replace(uri='test2'))
class GenericCopyTest(unittest.TestCase):
class GenericReplaceTest(unittest.TestCase):
def compare(self, orig, other):
self.assertEqual(orig, other)
self.assertEqual(id(orig), id(other))
def test_copying_track(self):
def test_replace_track(self):
track = Track()
self.compare(track, track.copy())
self.compare(track, track.replace())
def test_copying_artist(self):
def test_replace_artist(self):
artist = Artist()
self.compare(artist, artist.copy())
self.compare(artist, artist.replace())
def test_copying_album(self):
def test_replace_album(self):
album = Album()
self.compare(album, album.copy())
self.compare(album, album.replace())
def test_copying_playlist(self):
def test_replace_playlist(self):
playlist = Playlist()
self.compare(playlist, playlist.copy())
self.compare(playlist, playlist.replace())
def test_copying_track_with_basic_values(self):
def test_replace_track_with_basic_values(self):
track = Track(name='foo', uri='bar')
copy = track.copy(name='baz')
self.assertEqual('baz', copy.name)
self.assertEqual('bar', copy.uri)
other = track.replace(name='baz')
self.assertEqual('baz', other.name)
self.assertEqual('bar', other.uri)
def test_copying_track_with_missing_values(self):
def test_replace_track_with_missing_values(self):
track = Track(uri='bar')
copy = track.copy(name='baz')
self.assertEqual('baz', copy.name)
self.assertEqual('bar', copy.uri)
other = track.replace(name='baz')
self.assertEqual('baz', other.name)
self.assertEqual('bar', other.uri)
def test_copying_track_with_private_internal_value(self):
def test_replace_track_with_private_internal_value(self):
artist1 = Artist(name='foo')
artist2 = Artist(name='bar')
track = Track(artists=[artist1])
copy = track.copy(artists=[artist2])
self.assertIn(artist2, copy.artists)
other = track.replace(artists=[artist2])
self.assertIn(artist2, other.artists)
def test_copying_track_with_invalid_key(self):
def test_replace_track_with_invalid_key(self):
with self.assertRaises(TypeError):
Track().copy(invalid_key=True)
Track().replace(invalid_key=True)
def test_copying_track_to_remove(self):
track = Track(name='foo').copy(name=None)
def test_replace_track_to_remove(self):
track = Track(name='foo').replace(name=None)
self.assertFalse(hasattr(track, '_name'))
@ -209,7 +209,7 @@ class ArtistTest(unittest.TestCase):
def test_invalid_kwarg_with_name_matching_method(self):
with self.assertRaises(TypeError):
Artist(copy='baz')
Artist(replace='baz')
with self.assertRaises(TypeError):
Artist(serialize='baz')
@ -816,9 +816,9 @@ class TrackTest(unittest.TestCase):
self.assertEqual(track1, track2)
self.assertEqual(hash(track1), hash(track2))
def test_copy_can_reset_to_default_value(self):
def test_replace_can_reset_to_default_value(self):
track1 = Track(name='name1')
track2 = Track(name='name1', album=Album()).copy(album=None)
track2 = Track(name='name1', album=Album()).replace(album=None)
self.assertEqual(track1, track2)
self.assertEqual(hash(track1), hash(track2))
@ -943,7 +943,7 @@ class PlaylistTest(unittest.TestCase):
playlist = Playlist(
uri='an uri', name='a name', tracks=tracks,
last_modified=last_modified)
new_playlist = playlist.copy(uri='another uri')
new_playlist = playlist.replace(uri='another uri')
self.assertEqual(new_playlist.uri, 'another uri')
self.assertEqual(new_playlist.name, 'a name')
self.assertEqual(list(new_playlist.tracks), tracks)
@ -955,7 +955,7 @@ class PlaylistTest(unittest.TestCase):
playlist = Playlist(
uri='an uri', name='a name', tracks=tracks,
last_modified=last_modified)
new_playlist = playlist.copy(name='another name')
new_playlist = playlist.replace(name='another name')
self.assertEqual(new_playlist.uri, 'an uri')
self.assertEqual(new_playlist.name, 'another name')
self.assertEqual(list(new_playlist.tracks), tracks)
@ -968,7 +968,7 @@ class PlaylistTest(unittest.TestCase):
uri='an uri', name='a name', tracks=tracks,
last_modified=last_modified)
new_tracks = [Track(), Track()]
new_playlist = playlist.copy(tracks=new_tracks)
new_playlist = playlist.replace(tracks=new_tracks)
self.assertEqual(new_playlist.uri, 'an uri')
self.assertEqual(new_playlist.name, 'a name')
self.assertEqual(list(new_playlist.tracks), new_tracks)
@ -981,7 +981,7 @@ class PlaylistTest(unittest.TestCase):
playlist = Playlist(
uri='an uri', name='a name', tracks=tracks,
last_modified=last_modified)
new_playlist = playlist.copy(last_modified=new_last_modified)
new_playlist = playlist.replace(last_modified=new_last_modified)
self.assertEqual(new_playlist.uri, 'an uri')
self.assertEqual(new_playlist.name, 'a name')
self.assertEqual(list(new_playlist.tracks), tracks)