search

google-code-export / beets

Automatically exported from code.google.com/p/beets
MIT License
0 stars 0 forks source link

safer defaults for interactive tagger #214

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
This report starts from the premise that the absolute worst thing that beets 
can do is change a mostly-correct tag into a completely-incorrect tag. In the 
absence of a completely-reliable acoustic identifier, an album tagged in this 
way could only be corrected if the user happened to know the tracks were tagged 
wrong. This could be very difficult with instrumental music. Even worse, that 
person could submit their incorrectly tagged music to an acoustid id service, 
perpetuating the error.

This can happen in a few cases, but the simplest case is a near statistical 
match which is completely the wrong release. The results of "artist - title" 
searches also frequently return lists with zero correct items in the case where 
musicbrainz does not have the release.

The default in many interactive prompts, however, is either "[A]"pply or "1", 
which is the first item in the list. Output is also sometimes interleaved, 
especially if debug output is enabled. A user might therefore accidentally 
"[A]pply" or pick the first item in the list, which might be completely wrong.

I did a very hacky version of this in my local copy by defaulting to list item 
"99" and to "[S]" where possible, but would prefer a way (config option?) by 
which users could make every prompt default to "[S]"kip, which is always 
"[S]"afe. :D

Original issue reported on code.google.com by teh.awes...@gmail.com on 29 Jun 2011 at 4:26

GoogleCodeExporter commented 9 years ago 
To be honest I think this is more of a PEBKAC problem.

People shouldn't just hit return at a prompt if that's not what they should do 
/ are unsure what they're doing.

I'm not so convinced a statistical collision would happen either tbh but I 
guess it is possible.

Original comment by daniele.sluijters on 29 Jun 2011 at 4:51

GoogleCodeExporter commented 9 years ago 
I agree that no defaults can protect from all stupid user behavior. But hitting 
enter at a prompt accidentally is the sort of thing that happens all the time, 
especially when users are unfamiliar with the interface they are using. As 
mentioned at the top of the bug, I consider the negative results (correctly 
tagged files turned into permanently mis-tagged files that might be impossible 
to detect) to be so severe that they should be avoided at all costs. 

I would much rather my workflow be slightly slower (in summary : have to hit 
"A" and then "enter" a few more times?) than unsafe in this way.

While I agree there is a level of PEBKAC, beets currently makes it easy to make 
the following mistakes :

1) a prompt is displayed, but output (normal or debug) from other threads is 
also displayed, so you can't see that it's on the prompt line. in this case, it 
is quite natural to hit enter to redisplay the prompt, especially if the 
program does not support ctrl-l redraw. unfortunately, if you hit enter to 
redraw the prompt, you accidentally accept the default which you cannot see.

2) "1" (the first item in the list) makes no sense to me as a default in this 
case :
"
/raid/music/traktor/The Real Classics Of Chicago House Vol 2
Finding tags for album "Phuture - The Real Classics Of Chicago H".
Candidates:
1. Various Artists - The House Sound of Chicago: The Future House Edition [BCM 
Records, 1993] (39.2%)
[ snip ...]
# selection (default 1), Skip, Use as-is, as Tracks, Enter search,
enter Id, aBort?
"

The one match is extremely low confidence, and if the user hits enter twice 
they will write COMPLETELY wrong tags to their files. I would very much prefer 
the default for this prompt to ALWAYS be skip. Again, writing completely wrong 
tags to files is something I consider VERY SEVERE, ymmv. :)

Original comment by teh.awes...@gmail.com on 29 Jun 2011 at 5:03

GoogleCodeExporter commented 9 years ago 
Maybe the program should take the confidence into account. So if it is less 
than 90%, don't default to Apply but instead ask the user explicitely if he 
wants to commit those changes.

Original comment by fil...@gmail.com on 30 Jun 2011 at 8:22

GoogleCodeExporter commented 9 years ago 
I think a "careful prompts" config option makes sense. I could imagine it doing 
one of two things:
1. Make sure that "skip" is always the default.
2. Remove all defaults. That is, if you just hit Enter without typing a letter, 
it will complain and ask you again.
Does anyone have a preference between these two?

I could be convinced that it might make sense to make the default action "skip" 
or "as-is" on the multiple-candidate-selection screen; when you have multiple 
candidates, it's pretty rare that any of them actually make sense.

I am not as much a fan of having the default action switch around depending on 
the tagger's confidence, though. While it might be convenient sometimes, it 
would mean that you would have to look closely at each prompt to figure out 
what the default action will be *this* time.

Original comment by adrian.sampson on 30 Jun 2011 at 5:51

GoogleCodeExporter commented 9 years ago 
I prefer option 2.

If I want "skip" to always be the default, then with option 2 I can always just 
:

yes S | <beets commandline>

(FWIW, I agree, default shouldn't switch based on confidence. Too much risk of 
confusion, and doesn't protect against the actual case I want to prevent, which 
includes accidentally hitting enter when coming back to a terminal full of 
debug output..)

Original comment by teh.awes...@gmail.com on 1 Jul 2011 at 6:11

GoogleCodeExporter commented 9 years ago

Original comment by adrian.sampson on 16 Sep 2011 at 6:17

GoogleCodeExporter commented 9 years ago

Original comment by adrian.sampson on 24 Nov 2011 at 3:30

GoogleCodeExporter commented 9 years ago 
I agree, this is a PEBKAC problem.  But every now and then my fingers are 
quicker than my brain and I accidentally hit Enter and do an As-Is import of an 
untagged album.  

Before 1.0b12 all the songs in that album were renamed as ".../_/ (0000)/00. - 
.mp3", each file overwriting the previous one. In b12 I get to keep all the 
files :-) since they all get a unique name.  I use delete on import so this is 
a big improvement.

If the files have no tags at all, wouldn't skip be a better default then As-is?

Original comment by janerikd...@gmail.com on 17 Jan 2012 at 10:14

GoogleCodeExporter commented 9 years ago 
Issue 304 has been merged into this issue.

Original comment by adrian.sampson on 23 Jan 2012 at 10:24

GoogleCodeExporter commented 9 years ago

Original comment by adrian.sampson on 19 Feb 2012 at 10:40

GoogleCodeExporter commented 9 years ago

Original comment by adrian.sampson on 20 May 2012 at 10:51

GoogleCodeExporter commented 9 years ago

Original comment by adrian.sampson on 28 Nov 2012 at 4:29

GoogleCodeExporter commented 9 years ago 
I have implemented a solution and opened a pull request:

https://github.com/sampsyo/beets/pull/76

This PR also adds a new configuration option that prevents strong 
recommendation (and therefore auto-tagging) of partially matched albums and 
asks users to confirm them. The default value for this option maintains the 
current behaviour.

For the issue of safer defaults when the user is prompted to confirm, I have 
removed the default (so that accidentally pressing enter won't do anything) for:

- Partial matches, if `import: confirm_partial` setting is "yes".
- Matches that are below the medium recommendation threshold, but above the
  gap threshold.
- Matches that have no recommendation.
- Matches other than the best and auto-suggested match (e.g. user has chosen M 
to see more options, then chosen 2 or another number other than 1).

Original comment by real.hu...@mrmachine.net on 29 Jan 2013 at 1:51

GoogleCodeExporter commented 9 years ago 
Just adding to the above. I've left the default choice of 1 on the 
multi-release selection prompt because choosing to see more information for a 
release doesn't actually apply any changes, so this default is not dangerous, 
and the first choice is likely to be the first one you want to investigate even 
if it's likely that none are a good match.

If you do end up having to choose a release in this way, the next prompt where 
changes might actually be applied will always have no default, so again it will 
be safe if users just hit enter by mistake here.

As for the default changing based on the recommendation. If it were changing 
between apply, skip, import as-is, etc., then I would agree that having a 
default becomes almost a hindrance than a help. But I think that simply 
removing the default for suggested matches and no recommendation matches should 
provide a clear benefit (more safety) without making it any more difficult to 
match suggested medium or strongly recommended matches.

Original comment by real.hu...@mrmachine.net on 29 Jan 2013 at 3:38

GoogleCodeExporter commented 9 years ago 
Thanks to real.human for tackling this.

See the default_action config option in master.

Original comment by adrian.sampson on 31 Jan 2013 at 4:36