beets v1.5.0 is released

[sampsyo]

I absolutely cannot believe how long it took to get here, but we have release beets v1.5.0. But here it is! Yay! Onward and upward—it's at last time to drop a bunch of Python 2 stuff and move into a 3.6+ future.

Check out the changelog if you're into that sort of thing.

After this debacle, I want to institute regular releases—perhaps every 3 months? If you're interested in that, please post here and we'll get you into the "core team" chat to work out a plan.

[arogl]

@sampsyo I have done some more work here.

I ran it through with the following command:

pyupgrade --py36-plus

And cleaned up the flake8 errors.

The current tests are failing due to all of the bytes -> String conversion taking place as part of the support for 2 & 3.

More updates coming

[sampsyo]

Wow; that's awesome! I didn't know about pyupgrade; this seems like a really useful route. Thanks for giving it a shot!

[wisp3rwind]

Yeah, that seems to save a lot of the hard manual work indeed!

However: Is it possible to apply pyupgrades transformations one at a time, to record them as individual commits and postpone problematic ones? Given that you see failures due to the bytes/string handling, I would feel very reluctant to merge the huge patch from your tree as-is. Individual commits would also simplify review substantially in my opinion.

[arogl]

I have done all sorts of debugging to try and see where bytes are coming from, but the format passed in here is always bytes.

I have type cast the encode function in convert.py.

test/test_convert.py
beetsplug/convert.py#L218

test/test_convert.py F                                                                                                                                                                                                           [100%]

=============================================================================================================== FAILURES ===============================================================================================================
_______________________________________________________________________________________________ ImportConvertTest.test_delete_originals ________________________________________________________________________________________________

self = <test.test_convert.ImportConvertTest testMethod=test_delete_originals>

    def test_delete_originals(self):
        self.config['convert']['delete_originals'] = True
>       self.importer.run()

test/test_convert.py:122:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
beets/importer.py:341: in run
    pl.run_sequential()
beets/util/pipeline.py:383: in run_sequential
    list(self.pull())
beets/util/pipeline.py:460: in pull
    out = coro.send(msg)
beets/util/pipeline.py:191: in coro
    func(*(args + (task,)))
beets/importer.py:1531: in plugin_stage
    func(session, task)
beets/plugins.py:145: in wrapper
    return func(*args, **kwargs)
beetsplug/convert.py:183: in auto_convert
    par_map(lambda item: self.convert_on_import(config.lib, item),
beets/util/__init__.py:1048: in par_map
    pool.map(transform, items)
/usr/lib/python3.8/multiprocessing/pool.py:364: in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
/usr/lib/python3.8/multiprocessing/pool.py:771: in get
    raise self._value
/usr/lib/python3.8/multiprocessing/pool.py:125: in worker
    result = (True, func(*args, **kwds))
/usr/lib/python3.8/multiprocessing/pool.py:48: in mapstar
    return list(map(*args))
beetsplug/convert.py:183: in <lambda>
    par_map(lambda item: self.convert_on_import(config.lib, item),
beetsplug/convert.py:519: in convert_on_import
    self.encode(command, item.path, dest)
beetsplug/convert.py:218: in encode
    args[i] = Template(arg).safe_substitute({
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <string.Template object at 0x7fc8c7b32160>, mapping = {'dest': '/tmp/tmpdgp52ow2.mp3', 'source': '/tmp/tmpzrbxkbd_/import/album 0/track 0.mp3'}, kws = {}
convert = <function Template.safe_substitute.<locals>.convert at 0x7fc8c7b26af0>

    def safe_substitute(self, mapping=_sentinel_dict, /, **kws):
        if mapping is _sentinel_dict:
            mapping = kws
        elif kws:
            mapping = _ChainMap(kws, mapping)
        # Helper function for .sub()
        def convert(mo):
            named = mo.group('named') or mo.group('braced')
            if named is not None:
                try:
                    return str(mapping[named])
                except KeyError:
                    return mo.group()
            if mo.group('escaped') is not None:
                return self.delimiter
            if mo.group('invalid') is not None:
                return mo.group()
            raise ValueError('Unrecognized named group in pattern',
                             self.pattern)
>       return self.pattern.sub(convert, self.template)
E       TypeError: cannot use a string pattern on a bytes-like object

/usr/lib/python3.8/string.py:147: TypeError
--------------------------------------------------------------------------------------------------------- Captured stderr call ---------------------------------------------------------------------------------------------------------

[sampsyo]

Yes, the convert plugin needs some pretty special handling. The root of the problem is that we need to eventually produce bytes for the command, because that's what the OS will want to consume, but we need to do templating—which requires having a proper Unicode string. That's why there's a bunch of special-case handling in the manipulation of commands here.

[arogl]

I assume we would need to do the same for any plugin that calls out to any external command?

[wisp3rwind]

I assume we would need to do the same for any plugin that calls out to any external command?

Probably, but also note that ideally we share the infrastructure to do so in beets.util, see for example the work in https://github.com/beetbox/beets/pull/3930/files.

[arogl]

I could convert each directory as a commit and leave out the convert plugin for now.

[arogl]

OK, had another go here https://github.com/arogl/beets/tree/try2upgrade

Each directory updated via pyupgrade --py36-plus

Tox tests run on linux have the same failure/error count after each change.

Windows tests failing on the convert and replaygain tests.

[sampsyo]

Awesome; thanks for giving this a go!! FYI, I recently pushed some changes to #4018 that should clear up those convert (and maybe also replaygain?) tests. (By skipping the bad ones, but still…)

[arogl]

@sampsyo I had already pulled in all but the last 2 commits in #4018.

The failures on windows tox run relate to the bytes -> string conversions. I added some tests checking if on win32 and converting to a string. These then passed on win32 and Linux.

I'll do another push with the update to convert, as well as bring in your last 2 updates.

[arogl]

#4030 created.

All tests pass in my 2 test environments, wanted to test on github

[arogl]

To fix a lot of the test we will need to transition to pathlib.

#1409
#3704

#3690

As per #3704, we will need to write a lot more tests to ensure things work.

[wisp3rwind]

To fix a lot of the test we will need to transition to pathlib.

I'm not quite following here: Are you referring to tests that have always been failing, or rather tests that only fail (on Windows) after applying pyupgrade?

In the latter case, I'd really be wary about using pyupgrade in bulk like this: I'd very nice to split "stylistic" changes (such as string formatting syntax) from functional changes. Maybe we could make a feature request to the pyupgrade project to add a CLI switch to select the individual transformations (i.e. something like --features=no-printf-style,...)?

Also, I think we should not look into the pathlib transition until the general move to Python 3(.6) is mostly done. Otherwise, we'll probably have a hard time managing concurrent PRs which all touch the entire codebase. It might indeed make sense that I revisit #3690 before tackling pathlib such that we have a more consistent/correct starting point.

[arogl]

I have gone back and started with trying the upgrade/test on a per file basis.

[arogl]

For the next release, should we also look at resurrecting

#3694 - Add black as an auto formatter
#3715 - Switching to Poetry for dependency management

[sampsyo]

All true! Switching away from setup.py would be awesome. I have no objection to Poetry myself, although I also have really enjoyed using Flit, FWIW.

[wisp3rwind]

I agree, but as I said above: I think we should tackle one of these codebase-wide PRs at a time. We should also consider having a look at ongoing large PRs that might be very painful to merge/rebase after e.g. introducing black (the UI refactor and my own replaygain/convert PRs) and see whether they can be seen through in a reasonable time, and then only use black/pathlib afterwards.

[arogl]

Created #4033 for #3694, have also added an isort tree in #4034

Will look at Poetry soon

[matlads]

I absolutely cannot believe how long it took to get here, but we have release beets v1.5.0. But here it is! Yay! Onward and upward—it's at last time to drop a bunch of Python 2 stuff and move into a 3.6+ future.

Maybe post this announcement on the blog?