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

Add feature to vote on tags (#240) #241

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions musicbrainzngs/mbxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,17 @@ def make_tag_request(**kwargs):
e_xml = ET.SubElement(e_list, "{%s}%s" % (NS, entity_type.replace('_', '-')))
e_xml.set("{%s}id" % NS, e)
taglist = ET.SubElement(e_xml, "{%s}user-tag-list" % NS)
for tag in tags:
usertag_xml = ET.SubElement(taglist, "{%s}user-tag" % NS)
name_xml = ET.SubElement(usertag_xml, "{%s}name" % NS)
name_xml.text = tag
if isinstance(tags, dict):
for tag, vote in tags.items():
usertag_xml = ET.SubElement(taglist, "{%s}user-tag" % NS)
usertag_xml.set('vote', vote)
Copy link
Owner

Choose a reason for hiding this comment

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

we should raise an error if the vote parameter isn't a valid value

Copy link
Author

Choose a reason for hiding this comment

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

I implemented a ValueError

name_xml = ET.SubElement(usertag_xml, "{%s}name" % NS)
name_xml.text = tag
else:
for tag in tags:
usertag_xml = ET.SubElement(taglist, "{%s}user-tag" % NS)
name_xml = ET.SubElement(usertag_xml, "{%s}name" % NS)
name_xml.text = tag
if kwargs.keys():
raise TypeError("make_tag_request() got an unexpected keyword argument '%s'" % kwargs.popitem()[0])

Expand Down
6 changes: 4 additions & 2 deletions musicbrainzngs/musicbrainz.py
Original file line number Diff line number Diff line change
Expand Up @@ -1259,8 +1259,10 @@ def submit_tags(**kwargs):
"""Submit user tags.
Takes parameters named e.g. 'artist_tags', 'recording_tags', etc.,
and of the form:
{entity_id1: {tag1: vote, ...}, ...}
Copy link
Owner

Choose a reason for hiding this comment

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

The documentation should be updated to say what the value of "vote" can be - "upvote", "downvote", or "withdraw". I wonder if it also makes sense to support alternate values for for this? Integers? 1, 0, -1? Booleans? True, False, None?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your comments. I will try to update this branch soon.

And I need to check, if the code still works after all those years.

Copy link
Author

Choose a reason for hiding this comment

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

I improved the documentation now. Is it better now?

For the time being, I would stick with the official values for a vote ("upvote", "downvote", "withdraw"). At the moment, I'm not convinced to include a logic to convert boolean of interger values to this specific strings.

If you don't want to vote, you can use a list of tags instead:
{entity_id1: [tag1, ...], ...}
If you only have one tag for an entity you can use a string instead
If you only have one tag for an entity, you can use a string instead
of a list.

The user's tags for each entity will be set to that list, adding or
Expand All @@ -1269,7 +1271,7 @@ def submit_tags(**kwargs):
"""
for k, v in kwargs.items():
for id, tags in v.items():
kwargs[k][id] = tags if isinstance(tags, list) else [tags]
kwargs[k][id] = tags if isinstance(tags, (list, dict)) else [tags]

query = mbxml.make_tag_request(**kwargs)
return _do_mb_post("tag", query)
Expand Down