beetbox / mediafile

elegant audio file tagging
http://mediafile.readthedocs.io/
MIT License
100 stars 25 forks source link

Sync changes from beets.mediafile #6

Closed arcresu closed 5 years ago

arcresu commented 5 years ago

My process here was to create two clones of the beets repo, and with git filter-branch reduce them to only the changes to mediafile.py and its tests respectively. I then used git format-patch to bring the changes over since the date of the last sync, resolving some small conflicts manually.

There were quite a lot of commits and I don't claim to have inspected everything carefully. The tests do still pass locally however.

arcresu commented 5 years ago

The diffs from beets.mediafile look encouraging after this process:

diff --git a/../beets/beets/mediafile.py b/mediafile.py
index 32a32fe..99f9e01 100644
--- a/../beets/beets/mediafile.py
+++ b/mediafile.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# This file is part of beets.
+# This file is part of MediaFile.
 # Copyright 2016, Adrian Sampson.
 #
 # Permission is hereby granted, free of charge, to any person obtaining
diff --git a/../beets/test/test_mediafile.py b/test/test_mediafile.py
index 36a2c53..1a1192d 100644
--- a/../beets/test/test_mediafile.py
+++ b/test/test_mediafile.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# This file is part of beets.
+# This file is part of MediaFile.
 # Copyright 2016, Adrian Sampson.
 #
 # Permission is hereby granted, free of charge, to any person obtaining
@@ -26,7 +26,7 @@ import unittest
 from six import assertCountEqual

 from test import _common
-from beets.mediafile import MediaFile, Image, \
+from mediafile import MediaFile, Image, \
     ImageType, CoverArtField, UnreadableFileError

@@ -968,5 +968,6 @@ class MediaFieldTest(unittest.TestCase):
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

+
 if __name__ == '__main__':
     unittest.main(defaultTest='suite')
diff --git a/../beets/test/test_mediafile_edge.py b/test/test_mediafile_edge.py
index 8bf9e19..8d31de0 100644
--- a/../beets/test/test_mediafile_edge.py
+++ b/test/test_mediafile_edge.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# This file is part of beets.
+# This file is part of MediaFile.
 # Copyright 2016, Adrian Sampson.
 #
 # Permission is hereby granted, free of charge, to any person obtaining
@@ -24,7 +24,7 @@ import mutagen.id3

 from test import _common

-from beets import mediafile
+import mediafile
 import six
arcresu commented 5 years ago

Looks like some sort of problems with the CI config/infrastructure. Maybe this is something that was seen previously on beets itself?

sampsyo commented 5 years ago

Wow; awesome work!

I'm a little stumped by those CI failures. The job seems to fail right after trying to install Tox, but there's no obvious error in the log. It could be that stealing the Travis config from beets itself might work.

arcresu commented 5 years ago

After some trial and error porting CI config from beets, this is now working! I removed the mp3gain dependency since it doesn't exist in Xenial (or in Debian where it was removed) and it didn't appear to be necessary in the first place.

arcresu commented 5 years ago

Once this is merged it might be nice to push out a new release of MediaFile on PyPi so that we can start making beets depend on that. Would you like help with making/publishing that release when it comes time?

sampsyo commented 5 years ago

Yes; that sounds awesome! I'll need to add you (or others) as package administrators on PyPI. Do you have a PyPI username?

arcresu commented 5 years ago

Great! My username on PyPi is the same as on GitHub. My plan would be to make the mediafile release shortly after the next beets release in order to introduce the dependency early in the next beets release cycle.

I'm aware of at least one active PR in beets that affects mediafile. My plan would be to start by checking open PRs there that affect mediafile and try to help move them to completion if possible and assist with translating the changes to this repository.