From c2aeda7fee2639576b79a83614e062dae018fc2a Mon Sep 17 00:00:00 2001 From: Jacob Pavlock Date: Tue, 1 Nov 2022 13:44:31 -0700 Subject: [PATCH] fix: Track `audio_format` is now a property and not a field Use some regex query on path if you wish to query for it. For now, I'd like to keep track properties like bitrate, audio_format, etc. as generated properties since they require a data migration. I'd like to do that data migration all at once if possible. --- .../4279789f5739_audio_format_non_null.py | 53 ------------------- .../880c5a2d80ed_remove_audio_format_field.py | 26 +++++++++ docs/developers/api/core.rst | 2 +- docs/fields.rst | 1 - moe/library/track.py | 14 +++-- tests/conftest.py | 1 - tests/library/test_track.py | 12 +++-- tests/plugins/test_write.py | 1 - 8 files changed, 46 insertions(+), 64 deletions(-) delete mode 100644 alembic/versions/4279789f5739_audio_format_non_null.py create mode 100644 alembic/versions/880c5a2d80ed_remove_audio_format_field.py diff --git a/alembic/versions/4279789f5739_audio_format_non_null.py b/alembic/versions/4279789f5739_audio_format_non_null.py deleted file mode 100644 index 505ddd00..00000000 --- a/alembic/versions/4279789f5739_audio_format_non_null.py +++ /dev/null @@ -1,53 +0,0 @@ -"""audio format non null. - -Revision ID: 4279789f5739 -Revises: bf49ac6805f7 -Create Date: 2022-10-30 15:31:38.324821 - -""" -import mediafile -import sqlalchemy as sa -from sqlalchemy.orm import Session, declarative_base - -import moe -from alembic import op -from moe import config - -# revision identifiers, used by Alembic. -revision = "4279789f5739" -down_revision = "bf49ac6805f7" -branch_labels = None -depends_on = None - -Base = declarative_base() - - -class Track(Base): - __tablename__ = "track" - - _id = sa.Column(sa.Integer, primary_key=True) - path = sa.Column(moe.library.lib_item.PathType(), nullable=False, unique=True) - audio_format = sa.Column(sa.String, nullable=False) - - -def upgrade(): - if not config.CONFIG: # for tests that create tmp configs - config.Config(init_db=False) # read config to get library path - - # populate audio_format values before making it non-nullable - with Session(bind=op.get_bind()) as session: - tracks = session.query(Track).all() - for track in tracks: - audio_file = mediafile.MediaFile(track.path) - - track.audio_format = audio_file.type - - session.commit() - - with op.batch_alter_table("track", schema=None) as batch_op: - batch_op.alter_column("audio_format", existing_type=sa.String(), nullable=False) - - -def downgrade(): - with op.batch_alter_table("track", schema=None) as batch_op: - batch_op.alter_column("audio_format", existing_type=sa.String(), nullable=True) diff --git a/alembic/versions/880c5a2d80ed_remove_audio_format_field.py b/alembic/versions/880c5a2d80ed_remove_audio_format_field.py new file mode 100644 index 00000000..f4f4675e --- /dev/null +++ b/alembic/versions/880c5a2d80ed_remove_audio_format_field.py @@ -0,0 +1,26 @@ +"""remove audio_format field. + +Revision ID: 880c5a2d80ed +Revises: bf49ac6805f7 +Create Date: 2022-11-01 13:41:12.407819 + +""" +import sqlalchemy as sa + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "880c5a2d80ed" +down_revision = "bf49ac6805f7" +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table("track", schema=None) as batch_op: + batch_op.drop_column("audio_format") + + +def downgrade(): + with op.batch_alter_table("track", schema=None) as batch_op: + batch_op.add_column(sa.Column("audio_format", sa.VARCHAR(), nullable=True)) diff --git a/docs/developers/api/core.rst b/docs/developers/api/core.rst index 993c96f1..404747b4 100644 --- a/docs/developers/api/core.rst +++ b/docs/developers/api/core.rst @@ -27,7 +27,7 @@ Library .. automodule:: moe.library :members: - :exclude-members: cache_ok, path, year, catalog_num, genre, original_year + :exclude-members: cache_ok, path, year, catalog_num, genre, original_year, audio_format :show-inheritance: diff --git a/docs/fields.rst b/docs/fields.rst index 96226bd9..8b508ccc 100644 --- a/docs/fields.rst +++ b/docs/fields.rst @@ -14,7 +14,6 @@ Track Fields * ``albumartist`` - album artist * ``artist`` - track artist * ``artists`` - track artists [#f1]_ -* ``audio_format`` - aac, aiff, alac, ape, asf, dsf, flac, ogg, opus, mp3, mpc, wav, or wv * ``disc`` - disc number * ``genre`` - genre [#f1]_ * ``path`` - filesystem path of the track [#f3]_ diff --git a/moe/library/track.py b/moe/library/track.py index 3449a77a..dfbc3942 100644 --- a/moe/library/track.py +++ b/moe/library/track.py @@ -127,7 +127,6 @@ def read_custom_tags( track_fields["artist"] = audio_file.artist if audio_file.artists is not None: track_fields["artists"] = set(audio_file.artists) - track_fields["audio_format"] = audio_file.type track_fields["disc"] = audio_file.disc if audio_file.genres is not None: track_fields["genres"] = set(audio_file.genres) @@ -338,7 +337,6 @@ class Track(LibItem, SABase, MetaTrack): artists: Optional[set[str]] = cast( Optional[set[str]], MutableSet.as_mutable(Column(SetType, nullable=True)) ) - audio_format: str = cast(str, Column(String, nullable=False)) disc: int = cast(int, Column(Integer, nullable=False, default=1)) genres: Optional[set[str]] = cast( Optional[set[str]], MutableSet.as_mutable(Column(SetType, nullable=True)) @@ -388,7 +386,6 @@ def __init__( # set default values self.artist = self.albumartist - self.audio_format = self.path.suffix for key, value in kwargs.items(): setattr(self, key, value) @@ -476,10 +473,19 @@ def from_file(cls: type[T], track_path: Path, album: Optional[Album] = None) -> **track_fields, ) + @property + def audio_format(self) -> str: + """Returns the audio format of the track. + + One of ['aac', 'aiff', 'alac', 'ape', 'asf', 'dsf', 'flac', 'ogg', 'opus', + 'mp3', 'mpc', 'wav', 'wv']. + """ + return mediafile.MediaFile(self.path).type + @property def fields(self) -> set[str]: """Returns any editable, track-specific fields.""" - return super().fields.union({"audio_format", "path"}) + return super().fields.union({"path"}) def is_unique(self, other: "Track") -> bool: """Returns whether a track is unique in the library from ``other``.""" diff --git a/tests/conftest.py b/tests/conftest.py index c2fbe360..08adf8a3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -187,7 +187,6 @@ def track_factory( title=title, track_num=track_num, disc=disc, - audio_format=kwargs.pop("audio_format", "mp3"), **kwargs, ) diff --git a/tests/library/test_track.py b/tests/library/test_track.py index 37a89a7c..792c2099 100644 --- a/tests/library/test_track.py +++ b/tests/library/test_track.py @@ -239,12 +239,12 @@ def test_db_delete(self, tmp_session): def test_overwrite(self): """Fields are overwritten if the option is given.""" - track = track_factory(audio_format="1") - other_track = track_factory(audio_format="2") + track = track_factory(title="1") + other_track = track_factory(title="2") track.merge(other_track, overwrite=True) - assert track.audio_format == "2" + assert track.title == "2" class TestProperties: @@ -266,6 +266,12 @@ def test_set_genre(self): track.genre = "1; 2" assert track.genres == {"1", "2"} + def test_audio_format(self): + """We can get the audio format of a track.""" + track = track_factory(exists=True) + + assert track.audio_format == "mp3" + class TestListDuplicates: """List fields should not cause duplicate errors (just merge silently).""" diff --git a/tests/plugins/test_write.py b/tests/plugins/test_write.py index 5a44a4da..3073a00b 100644 --- a/tests/plugins/test_write.py +++ b/tests/plugins/test_write.py @@ -107,7 +107,6 @@ def test_write_tags(self, tmp_config): assert new_track.albumartist == albumartist assert new_track.artist == artist assert new_track.artists == artists - assert new_track.audio_format == "mp3" assert new_track.disc == disc assert new_track.genres == genres assert new_track.title == title