Rename error

[discopatrick]

As discussed at #2517 (comment), the way we use InvalidQueryArgumentTypeError is more akin to a ValueError than a TypeError. For example, we try to parse a string as an int, float, or date, but the parsing fails - there was nothing wrong with the Type of the variable (string), but its contents were not parseable into the type we wanted - there was a problem with the value of the string.

Thus InvalidQueryArgumentTypeError is renamed to InvalidQueryArgumentValueError in this pull request.

[mention-bot]

@discopatrick, thanks for your PR! By analyzing the history of the files in this pull request, we identified @geigerzaehler, @PierreRust and @sampsyo to be potential reviewers.

[Freso]

Can you (re)write you commit messages in the imperative voice and capitalise them?

Would also be great if you would state your reasoning for the changes in your commits directly in the commit message. 10 years from now, beets's source might not be tracked on GitHub anymore and any information and reasoning you've added to the pull request but not in the commits might be lost.

(The code itself looks fine, at a glance.)

[discopatrick]

Hi @Freso, sure, I'm happy to bring my commit messages in line with the repository convention.

I'm trying to follow this guide to do this: https://help.github.com/articles/changing-a-commit-message/#amending-older-or-multiple-commit-messages

...but I'm having some problems. Here's what I'm doing:

PHs-MacBook-Pro:beets patrick$ git checkout rename-error
Already on 'rename-error'
Your branch is up-to-date with 'origin/rename-error'.
PHs-MacBook-Pro:beets patrick$ git rebase -i HEAD~4

Then I choose which commits to rename (there are other commits from merged branches here):

reword 398c44d renames InvalidQueryArgumentTypeError to InvalidQueryArgumentValueError
pick 6515f9c changes date validation method to try each format until a match is found
pick 713c00a reverts order of precisions from broadest to narrowest
pick 85adbd1 gives variable the better name of ‘ordinal’
pick 4dbc413 consolidates the declaration and incrementing of `ordinal` into one line
reword dc842c0 corrects error type
reword 067bc30 passes flake8

I can get as far as rewriting the most recent message, but then:

[detached HEAD 88632d3] Rename 'InvalidQueryArgumentTypeError' to 'InvalidQueryArgumentValueError'
 4 files changed, 20 insertions(+), 20 deletions(-)
error: could not apply 6515f9c... changes date validation method to try each format until a match is found

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
Could not apply 6515f9c55a45ff870375bae112624c23201d1373... changes date validation method to try each format until a match is found

I think this is something to do with the fact that there are merged branches here (I merged master into my feature branch just before my most recent commit). I'm guessing this means I can't rewrite those commit messages as I'd have to rewrite the history of other branches too? (Edited to add: those commits are also already merged into master.) If this is the case, shall I just rewrite the most recent commit message (together with a "why" explanation in the body) and then remember to apply the repo convention in future?

[Freso]

Yes, don't rewrite history for commits that are already in beets' master. (However, your two commits prior to your merge are not in the current master, but I mostly wanted to point it out so you can do better for future commits, so just fixing up the latest commit is okay with me. :))

[discopatrick]

I managed to find a way to edit commit messages that are prior to a merge! I had to add the -p flag, or --preserve-merges - i.e.: git rebase -i -p HEAD~4. The rebase process is still interrupted, but this time I was presented with merge conflicts that I simply re-resolved. I could then finish the rebase with git rebase --continue.

[sampsyo]

This looks great; thanks for doing the renaming! I would have even been fine with the shorter name InvalidQueryArgumentError, but this one is perfectly fine too.

[discopatrick]

@sampsyo @Freso is there any more action required before this PR can me merged?

[sampsyo]

Looks like you've addressed @Freso's suggestions about commit messages, so I'll merge this now.

Thank you for taking care of this! ✨ 🐟 ❗