Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always have a non-degenerate path to display for *Task #1475

Closed
wants to merge 1 commit into from

Conversation

jamesob
Copy link

@jamesob jamesob commented May 25, 2015

As I started to run an import, I noticed both in the autotagging prompts and logfile that certain filenames were instead printed as None, e.g.

None (1 items)
Finding tags for album "MF DOOM - MM..FOOD".
Candidates:

It turns out that ImportTask.paths is referenced a bunch of places as an assumed-truthy value. To support these references, I've added a property called paths_for_display which will fall back to the task's items should its paths be falsey.

After changes, we have

/Users/job/Music/collection/4ize/MM..Food_/Guinnesses.mp3 (1 items)
Finding tags for album "MF DOOM - MM..FOOD".
Candidates:

Please advise in terms of any tests that need to be updated.

@sampsyo
Copy link
Member

sampsyo commented May 25, 2015

Thanks for looking into this!

I'm trying to try to understand the root of the problem (i.e., why does task.paths ever become None?), and it looks like this happens when using album-grouping. Does that seem right to you? That is, does the None only arise when you choose the "group albums" option (or else you have that set in your config file)?

@jamesob
Copy link
Author

jamesob commented May 25, 2015

@sampsyo yeah, that's certainly consistent with my config.

@jamesob
Copy link
Author

jamesob commented May 25, 2015

 ⇨  beet config

directory: ~/Dropbox/music/collection

import:
    write: yes
    copy: yes
    move: no
    incremental: yes
    group_albums: yes
    log: ~/Dropbox/music/beets/beets.log
threaded: yes

ui:
    color: yes
library: ~/Dropbox/music/beets/library.db

@sampsyo
Copy link
Member

sampsyo commented May 25, 2015

Great. Thanks for the extra detail.

I think it was an oversight not to set the task's paths in the first place. The paths field has two purposes: display/logging, as you found here, and also progress/history tracking. For both purposes, I'm pretty sure we want grouped-album tasks to have paths that reflect the individual files involved. So I'm pushing a change now that initializes the field with that in mind.

See also #825, which brought up the same logging issue.

Would you mind trying this out to make sure I didn't break anything?

@sampsyo sampsyo closed this in 7f5c274 May 25, 2015
LordSputnik pushed a commit to LordSputnik/beets that referenced this pull request Jul 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants