Refactored move functions for clarity according to #2682

[zigarrre]

The move functions in library.py and manipule_files in importer.py where
changed to use a single parameter for the file operation instead of
multiple boolean flags.

A typo in the documentation of the Album.move and Item.move functions
confusing True and False when describing the store parameter was fixed
as well.

[sampsyo]

Fantastic! The new interfaces look great. And the improvements to various docstrings are welcome changes too.

I did a full review and everything looks good. I added just one comment inline reflecting a flake8 warning.

[sampsyo]

flake8 has a (pretty annoying) suggestion here: it wants the ands to go on the line preceding, rather than the line following. Like this:

                if (operation != MoveOperation.MOVE and
                        self.replaced_items[item] and
                        session.lib.directory in util.ancestry(old_path)):

[zigarrre]

I know about that flake8 warning, I did see it when running tox. Though I decided to ignore it as it is a bug. [1] [2] [3] [4]

PEP8 was updated to allow both styles and even states breaking before (like I did) as the preferred style for new code. [5]

For those reasons I suggest we ignore this bogus warning by adding the following line to the flake8 configuration in tox.ini:

ignore = W503

[sampsyo]

Got it; thanks for digging! I agree: let’s ignore this warning. Would you mind adding that ignore flag to this PR?

[zigarrre]

I did some more digging and found out a few more things. The W503 bug is already fixed as it's added to the default ignore list but you have a ignore option configured in setup.cfg and if that's the case the default ignores aren't respected anymore. This is in my opinion pretty stupid behavior but a flake8 thing I can't do anything about. That means though that if you specify a ignore list you have to list all default ignores as well. So I updated the list in setup.cfg and now the tests pass without problems.

On a side note: It might be worth thinking about cleaning up the tox and flake8 config. The multiple flake8 commands with different --min-version seem rather useless to me (especially as only the 2.7 one is run and min-version=2.7 is specified in setup.cfg as well). Though this is the first time I'm using tox and flake8 so I might just not understand something here.

[sampsyo]

Indeed; that is a little dumb. Thanks for investigating, and for adding the seemingly-redundant ignore to our config. I’ve merged the PR—it’s looking great!

It’s true that we’re a little more thorough than we probably need to be with our tox config for flake8; I’m not sure if the multiple versions for flake8 have ever caught anything that one run would not. However, my general philosophy is that having more tox configs that we really need is probably OK—as long as we keep our default set of environments (and the Travis runs) trimmed to a reasonable set. But if it’s getting unnecessarily confusing, I wouldn’t object to a rethink there.