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

#2347: First hack of ignoring already tagged items #2349

Merged
merged 7 commits into from
Dec 28, 2016

Conversation

SusannaMaria
Copy link
Contributor

Improve the speed of fetching acousticbrainz data. Ignore already existing data.


if not force:
mood_str = item.get('mood_acoustic', u'')
if len(mood_str) != 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or simply
if mood_str:

@@ -118,7 +119,13 @@ def commands(self):

def func(lib, opts, args):
items = lib.items(ui.decargs(args))
self._fetch_info(items, ui.should_write())
self._fetch_info(items, ui.should_write(),opts.force_refetch or self.config['force'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

,opts.force_refetch => , opts.force_refetch

Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fantastic; thank you! @Kraymer has a couple of small suggestions, and I found one addition that could be useful here. And would you mind adding just a sentence or two about the new option to the plugin's documentation page (docs/plugins/acousticbrainz.rst)?

@@ -118,7 +119,13 @@ def commands(self):

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might need something like this to make the command-line flag work:

cmd.parser.add_option(
    u'-f', u'--force', dest='force_refetch',
    action='store_true', default=False,
    help=u're-download data if present',
)

@sampsyo sampsyo merged commit 7e1e31b into beetbox:master Dec 28, 2016
sampsyo added a commit that referenced this pull request Dec 28, 2016
#2347: First hack of ignoring already tagged items
sampsyo added a commit that referenced this pull request Dec 28, 2016
sampsyo added a commit that referenced this pull request Dec 28, 2016
sampsyo added a commit that referenced this pull request Dec 28, 2016
@sampsyo
Copy link
Member

sampsyo commented Dec 28, 2016

Looks perfect! Thank you! ✨ 🚀 🤖

I've merged this for inclusion in the next release.

sampsyo added a commit that referenced this pull request Dec 28, 2016
@SusannaMaria SusannaMaria deleted the #2347 branch December 31, 2016 12:04
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.

3 participants