From 352de135cd12dfccf8376cfaf53078c9611d821b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 9 Feb 2015 01:11:56 +0100 Subject: [PATCH] core: Deprecate properties (fixes #952) --- docs/changelog.rst | 4 ++ mopidy/core/actor.py | 12 +++- mopidy/core/playback.py | 123 +++++++++++++++++++++++++++++---------- mopidy/core/playlists.py | 10 +++- mopidy/core/tracklist.py | 120 +++++++++++++++++++++++++++++--------- 5 files changed, 203 insertions(+), 66 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 47bbe0b3..1a9eb33d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,10 @@ v0.20.0 (UNRELEASED) **Core API** +- Deprecate all properties in the core API. The previously undocumented getter + and setter methods are now the official API. This aligns the Python API with + the WebSocket/JavaScript API. (Fixes: :issue:`952`) + - Added :class:`mopidy.core.HistoryController` which keeps track of what tracks have been played. (Fixes: :issue:`423`, PR: :issue:`803`) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index d53e4d38..370c9e0d 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -62,19 +62,27 @@ class Core( self.audio = audio def get_uri_schemes(self): + """Get list of URI schemes we can handle""" futures = [b.uri_schemes for b in self.backends] results = pykka.get_all(futures) uri_schemes = itertools.chain(*results) return sorted(uri_schemes) uri_schemes = property(get_uri_schemes) - """List of URI schemes we can handle""" + """ + .. deprecated:: 0.20 + Use :meth:`get_uri_schemes` instead. + """ def get_version(self): + """Get version of the Mopidy core API""" return versioning.get_version() version = property(get_version) - """Version of the Mopidy core API""" + """ + .. deprecated:: 0.20 + Use :meth:`get_version` instead. + """ def reached_end_of_stream(self): self.playback.on_end_of_track() diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 3b359b28..72a9207c 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -19,6 +19,8 @@ class PlaybackController(object): self.backends = backends self.core = core + self._current_tl_track = None + self._current_metadata_track = None self._state = PlaybackState.STOPPED self._volume = None self._mute = False @@ -34,39 +36,80 @@ class PlaybackController(object): # Properties def get_current_tl_track(self): - return self.current_tl_track + """Get the currently playing or selected track. - current_tl_track = None + Returns a :class:`mopidy.models.TlTrack` or :class:`None`. + """ + return self._current_tl_track + + def set_current_tl_track(self, value): + """Set the currently playing or selected track. + + *Internal:* This is only for use by Mopidy's test suite. + """ + self._current_tl_track = value + + current_tl_track = property(get_current_tl_track, set_current_tl_track) """ - The currently playing or selected :class:`mopidy.models.TlTrack`, or - :class:`None`. + .. deprecated:: 0.20 + Use :meth:`get_current_tl_track` instead. """ def get_current_track(self): - return self.current_tl_track and self.current_tl_track.track + """ + Get the currently playing or selected track. + + Extracted from :meth:`get_current_tl_track` for convenience. + + Returns a :class:`mopidy.models.Track` or :class:`None`. + """ + tl_track = self.get_current_tl_track() + if tl_track is not None: + return tl_track.track current_track = property(get_current_track) """ - The currently playing or selected :class:`mopidy.models.Track`. - - Read-only. Extracted from :attr:`current_tl_track` for convenience. + .. deprecated:: 0.20 + Use :meth:`get_current_track` instead. """ def get_current_metadata_track(self): - return self.current_metadata_track + """ + Get a :class:`mopidy.models.TlTrack` with updated metadata for the + currently playing track. - current_metadata_track = None + Returns :class:`None` if no track is currently playing. + """ + return self._current_metadata_track + + current_metadata_track = property(get_current_metadata_track) """ - A :class:`mopidy.models.TlTrack` with updated metadata for the currently - playing track. - - :class:`None` if no track is currently playing. + .. deprecated:: 0.20 + Use :meth:`get_current_metadata_track` instead. """ def get_state(self): + """Get The playback state.""" + return self._state def set_state(self, new_state): + """Set the playback state. + + Must be :attr:`PLAYING`, :attr:`PAUSED`, or :attr:`STOPPED`. + + Possible states and transitions: + + .. digraph:: state_transitions + + "STOPPED" -> "PLAYING" [ label="play" ] + "STOPPED" -> "PAUSED" [ label="pause" ] + "PLAYING" -> "STOPPED" [ label="stop" ] + "PLAYING" -> "PAUSED" [ label="pause" ] + "PLAYING" -> "PLAYING" [ label="play" ] + "PAUSED" -> "PLAYING" [ label="resume" ] + "PAUSED" -> "STOPPED" [ label="stop" ] + """ (old_state, self._state) = (self.state, new_state) logger.debug('Changing state: %s -> %s', old_state, new_state) @@ -74,23 +117,12 @@ class PlaybackController(object): state = property(get_state, set_state) """ - The playback state. Must be :attr:`PLAYING`, :attr:`PAUSED`, or - :attr:`STOPPED`. - - Possible states and transitions: - - .. digraph:: state_transitions - - "STOPPED" -> "PLAYING" [ label="play" ] - "STOPPED" -> "PAUSED" [ label="pause" ] - "PLAYING" -> "STOPPED" [ label="stop" ] - "PLAYING" -> "PAUSED" [ label="pause" ] - "PLAYING" -> "PLAYING" [ label="play" ] - "PAUSED" -> "PLAYING" [ label="resume" ] - "PAUSED" -> "STOPPED" [ label="stop" ] + .. deprecated:: 0.20 + Use :meth:`get_state` and :meth:`set_state` instead. """ def get_time_position(self): + """Get time position in milliseconds.""" backend = self._get_backend() if backend: return backend.playback.get_time_position().get() @@ -98,9 +130,18 @@ class PlaybackController(object): return 0 time_position = property(get_time_position) - """Time position in milliseconds.""" + """ + .. deprecated:: 0.20 + Use :meth:`get_time_position` instead. + """ def get_volume(self): + """Get the volume. + + Integer in range [0..100] or :class:`None` if unknown. + + The volume scale is linear. + """ if self.mixer: return self.mixer.get_volume().get() else: @@ -108,6 +149,12 @@ class PlaybackController(object): return self._volume def set_volume(self, volume): + """Set the volume. + + The volume is defined as an integer in range [0..100]. + + The volume scale is linear. + """ if self.mixer: self.mixer.set_volume(volume) else: @@ -115,11 +162,16 @@ class PlaybackController(object): self._volume = volume volume = property(get_volume, set_volume) - """Volume as int in range [0..100] or :class:`None` if unknown. The volume - scale is linear. + """ + .. deprecated:: 0.20 + Use :meth:`get_volume` and :meth:`set_volume` instead. """ def get_mute(self): + """Get mute state. + + :class:`True` if muted, :class:`False` otherwise. + """ if self.mixer: return self.mixer.get_mute().get() else: @@ -127,6 +179,10 @@ class PlaybackController(object): return self._mute def set_mute(self, value): + """Set mute state. + + :class:`True` to mute, :class:`False` to unmute. + """ value = bool(value) if self.mixer: self.mixer.set_mute(value) @@ -135,7 +191,10 @@ class PlaybackController(object): self._mute = value mute = property(get_mute, set_mute) - """Mute state as a :class:`True` if muted, :class:`False` otherwise""" + """ + .. deprecated:: 0.20 + Use :meth:`get_mute` and :meth:`set_mute` instead. + """ # Methods diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index c896bfa7..16b29b85 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -15,6 +15,11 @@ class PlaylistsController(object): self.backends = backends self.core = core + """ + Get the available playlists. + + Returns a list of :class:`mopidy.models.Playlist`. + """ def get_playlists(self, include_tracks=True): futures = [b.playlists.playlists for b in self.backends.with_playlists.values()] @@ -26,9 +31,8 @@ class PlaylistsController(object): playlists = property(get_playlists) """ - The available playlists. - - Read-only. List of :class:`mopidy.models.Playlist`. + .. deprecated:: 0.20 + Use :meth:`get_playlists` instead. """ def create(self, name, uri_scheme=None): diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index f9560a13..5f7ddba1 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -26,32 +26,42 @@ class TracklistController(object): # Properties def get_tl_tracks(self): + """Get tracklist as list of :class:`mopidy.models.TlTrack`.""" return self._tl_tracks[:] tl_tracks = property(get_tl_tracks) """ - List of :class:`mopidy.models.TlTrack`. - - Read-only. + .. deprecated:: 0.20 + Use :meth:`get_tl_tracks` instead. """ def get_tracks(self): + """Get tracklist as list of :class:`mopidy.models.Track`.""" return [tl_track.track for tl_track in self._tl_tracks] tracks = property(get_tracks) """ - List of :class:`mopidy.models.Track` in the tracklist. - - Read-only. + .. deprecated:: 0.20 + Use :meth:`get_tracks` instead. """ def get_length(self): + """Get length of the tracklist.""" return len(self._tl_tracks) length = property(get_length) - """Length of the tracklist.""" + """ + .. deprecated:: 0.20 + Use :meth:`get_length` instead. + """ def get_version(self): + """ + Get the tracklist version. + + Integer which is increased every time the tracklist is changed. Is not + reset before Mopidy is restarted. + """ return self._version def _increase_version(self): @@ -61,32 +71,57 @@ class TracklistController(object): version = property(get_version) """ - The tracklist version. - - Read-only. Integer which is increased every time the tracklist is changed. - Is not reset before Mopidy is restarted. + .. deprecated:: 0.20 + Use :meth:`get_version` instead. """ def get_consume(self): + """Get consume mode. + + :class:`True` + Tracks are removed from the tracklist when they have been played. + :class:`False` + Tracks are not removed from the tracklist. + """ return getattr(self, '_consume', False) def set_consume(self, value): + """Set consume mode. + + :class:`True` + Tracks are removed from the tracklist when they have been played. + :class:`False` + Tracks are not removed from the tracklist. + """ if self.get_consume() != value: self._trigger_options_changed() return setattr(self, '_consume', value) consume = property(get_consume, set_consume) """ - :class:`True` - Tracks are removed from the tracklist when they have been played. - :class:`False` - Tracks are not removed from the tracklist. + .. deprecated:: 0.20 + Use :meth:`get_consume` and :meth:`set_consume` instead. """ def get_random(self): + """Get random mode. + + :class:`True` + Tracks are selected at random from the tracklist. + :class:`False` + Tracks are played in the order of the tracklist. + """ return getattr(self, '_random', False) def set_random(self, value): + """Set random mode. + + :class:`True` + Tracks are selected at random from the tracklist. + :class:`False` + Tracks are played in the order of the tracklist. + """ + if self.get_random() != value: self._trigger_options_changed() if value: @@ -96,44 +131,71 @@ class TracklistController(object): random = property(get_random, set_random) """ - :class:`True` - Tracks are selected at random from the tracklist. - :class:`False` - Tracks are played in the order of the tracklist. + .. deprecated:: 0.20 + Use :meth:`get_random` and :meth:`set_random` instead. """ def get_repeat(self): + """ + Get repeat mode. + + :class:`True` + The tracklist is played repeatedly. + :class:`False` + The tracklist is played once. + """ return getattr(self, '_repeat', False) def set_repeat(self, value): + """ + Set repeat mode. + + To repeat a single track, set both ``repeat`` and ``single``. + + :class:`True` + The tracklist is played repeatedly. + :class:`False` + The tracklist is played once. + """ + if self.get_repeat() != value: self._trigger_options_changed() return setattr(self, '_repeat', value) repeat = property(get_repeat, set_repeat) """ - :class:`True` - The tracklist is played repeatedly. To repeat a single track, select - both :attr:`repeat` and :attr:`single`. - :class:`False` - The tracklist is played once. + .. deprecated:: 0.20 + Use :meth:`get_repeat` and :meth:`set_repeat` instead. """ def get_single(self): + """ + Get single mode. + + :class:`True` + Playback is stopped after current song, unless in ``repeat`` mode. + :class:`False` + Playback continues after current song. + """ return getattr(self, '_single', False) def set_single(self, value): + """ + Set single mode. + + :class:`True` + Playback is stopped after current song, unless in ``repeat`` mode. + :class:`False` + Playback continues after current song. + """ if self.get_single() != value: self._trigger_options_changed() return setattr(self, '_single', value) single = property(get_single, set_single) """ - :class:`True` - Playback is stopped after current song, unless in :attr:`repeat` - mode. - :class:`False` - Playback continues after current song. + .. deprecated:: 0.20 + Use :meth:`get_single` and :meth:`set_single` instead. """ # Methods