{"action":"created","issue":{"url":"https://api.github.com/repos/alastair/python-musicbrainzngs/issues/190","labels_url":"https://api.github.com/repos/alastair/python-musicbrainzngs/issues/190/labels{/name}","comments_url":"https://api.github.com/repos/alastair/python-musicbrainzngs/issues/190/comments","events_url":"https://api.github.com/repos/alastair/python-musicbrainzngs/issues/190/events","html_url":"https://github.com/alastair/python-musicbrainzngs/pull/190","id":129394377,"number":190,"title":"Read xml attributes and values from <attribute-list> elements","user":{"login":"alastair","id":19217,"avatar_url":"https://avatars.githubusercontent.com/u/19217?v=3","gravatar_id":"","url":"https://api.github.com/users/alastair","html_url":"https://github.com/alastair","followers_url":"https://api.github.com/users/alastair/followers","following_url":"https://api.github.com/users/alastair/following{/other_user}","gists_url":"https://api.github.com/users/alastair/gists{/gist_id}","starred_url":"https://api.github.com/users/alastair/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alastair/subscriptions","organizations_url":"https://api.github.com/users/alastair/orgs","repos_url":"https://api.github.com/users/alastair/repos","events_url":"https://api.github.com/users/alastair/events{/privacy}","received_events_url":"https://api.github.com/users/alastair/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/alastair/python-musicbrainzngs/milestones/7","html_url":"https://github.com/alastair/python-musicbrainzngs/milestones/0.6","labels_url":"https://api.github.com/repos/alastair/python-musicbrainzngs/milestones/7/labels","id":561032,"number":7,"title":"0.6","description":"","creator":{"login":"alastair","id":19217,"avatar_url":"https://avatars.githubusercontent.com/u/19217?v=3","gravatar_id":"","url":"https://api.github.com/users/alastair","html_url":"https://github.com/alastair","followers_url":"https://api.github.com/users/alastair/followers","following_url":"https://api.github.com/users/alastair/following{/other_user}","gists_url":"https://api.github.com/users/alastair/gists{/gist_id}","starred_url":"https://api.github.com/users/alastair/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alastair/subscriptions","organizations_url":"https://api.github.com/users/alastair/orgs","repos_url":"https://api.github.com/users/alastair/repos","events_url":"https://api.github.com/users/alastair/events{/privacy}","received_events_url":"https://api.github.com/users/alastair/received_events","type":"User","site_admin":false},"open_issues":11,"closed_issues":33,"state":"open","created_at":"2014-02-06T15:20:36Z","updated_at":"2016-01-28T10:03:54Z","due_on":"2014-02-28T08:00:00Z","closed_at":null},"comments":1,"created_at":"2016-01-28T09:29:44Z","updated_at":"2016-01-30T15:37:56Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/alastair/python-musicbrainzngs/pulls/190","html_url":"https://github.com/alastair/python-musicbrainzngs/pull/190","diff_url":"https://github.com/alastair/python-musicbrainzngs/pull/190.diff","patch_url":"https://github.com/alastair/python-musicbrainzngs/pull/190.patch"},"body":"Last part of #142!\r\n\r\nPreviously, with <attribute> values, we only read the text value,\r\nhowever now these tags can have value=\"\" and credited-as=\"\"\r\nattributes, which we now read. We add these to a separate list of\r\ndictionaries to keep backwards compatibility with the existing\r\nattribute-list element.\r\n\r\nNeed to add some documentation saying to now use the new element\r\n\r\nI'm not sure about the implementation of parse_relation_attribute_element. I thought that these attributes could be anything, but looking at the mmd I see there are only two options - `value` and `credited-as` (https://bitbucket.org/metabrainz/mmd-schema/src/master/schema/musicbrainz_mmd-2.0.rng?fileviewer=file-view-default#musicbrainz_mmd-2.0.rng-2311:2317). Perhaps we could just adapt `parse_work_attribute` to support the three attributes (including `type`) and adjust it as new changes are made to mmd. The current system means we have forward compatibility, but that hasn't stopped us parsing this data strictly in the past.\r\n\r\nI added the `attribute-dict` item to works as well, although it seems we don't have a backwards compatibility problem here, as this list has always been a dict. Should we remove `attribute-dict` then? It's strange that we have `attribute-list` appearing in two places (relations, and works), with different meanings.\r\n\r\nThe name `attribute-dict` still doesn't make me feel happy, especially that it's a list of dicts. Do we call it `attribute-dicts`? Perhaps we could just call it `attributes` - it doesn't reflect the XML as strictly as we normally follow it, but it is a lot simpler to read.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/alastair/python-musicbrainzngs/issues/comments/177212002","html_url":"https://github.com/alastair/python-musicbrainzngs/pull/190#issuecomment-177212002","issue_url":"https://api.github.com/repos/alastair/python-musicbrainzngs/issues/190","id":177212002,"user":{"login":"JonnyJD","id":158238,"avatar_url":"https://avatars.githubusercontent.com/u/158238?v=3","gravatar_id":"","url":"https://api.github.com/users/JonnyJD","html_url":"https://github.com/JonnyJD","followers_url":"https://api.github.com/users/JonnyJD/followers","following_url":"https://api.github.com/users/JonnyJD/following{/other_user}","gists_url":"https://api.github.com/users/JonnyJD/gists{/gist_id}","starred_url":"https://api.github.com/users/JonnyJD/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JonnyJD/subscriptions","organizations_url":"https://api.github.com/users/JonnyJD/orgs","repos_url":"https://api.github.com/users/JonnyJD/repos","events_url":"https://api.github.com/users/JonnyJD/events{/privacy}","received_events_url":"https://api.github.com/users/JonnyJD/received_events","type":"User","site_admin":false},"created_at":"2016-01-30T15:37:56Z","updated_at":"2016-01-30T15:37:56Z","body":"First of all: I also find it really strange that the `attribute-list` from the web service works completely different for works and relations.\r\nExample for works (*not* a relation):\r\nhttp://musicbrainz.org/ws/2/work/b85845e2-38fc-3092-a992-d373e7d517e9\r\n```\r\n<attribute-list>\r\n  <attribute type=\"Key\">G-flat major</attribute>\r\n</attribute-list>\r\n```\r\nExample for artist-relations (member of band):\r\nhttp://musicbrainz.org/ws/2/artist/57961a97-3796-4bf7-9f02-a985a8979ae9?inc=artist-rels\r\n```\r\n<attribute-list>\r\n  <attribute>founder</attribute>\r\n</attribute-list>\r\n```\r\nseries-relations(part of):\r\nmusicbrainz.org/ws/2/work/72c9aad2-3c95-4e3e-8a01-3974f8fef8eb?inc=series-rels+attr\r\n```\r\n<attribute-list>\r\n  <attribute value=\"BuxWV 1\">number</attribute>\r\n</attribute-list>\r\n```\r\n\r\nFor some relations the attribute-list isn't of much help without the value:\r\n```\r\n'attribute-list': ['number']\r\n```\r\nFor others (booleans) it is (not listed when false):\r\n```\r\n'attribute-list': ['founder']\r\n```\r\nSo the attribute-list is possibly even used and we really can't change how we return that.\r\n\r\nNote that for all relation attributes the meaning is backwards like for work attributes.\r\nFor works the value is in the text and for relations the name of the attribute is in the text Oo.\r\n\r\nUsing `attributes`, rather than `attribute-dict` sounds better, but from the name it isn't clear what's the difference to `attribute-list`. Both are lists.\r\nI would use *`attribute-dicts`* note the plural s.\r\n\r\nNot sure about adding `attribute-dicts` to works. I probably wouldn't because that would indicate it works the same as the 'attribute-dicts` from relations, but it doesn't. \r\nCompare (both relations) for `number=BuxWV 1`:\r\n```\r\n{'value': 'BuxWV 1', 'attribute': 'number'}\r\n```\r\nand (work attributes) for `Key=G-flat major`\r\n```\r\n{'type': 'Key', 'attribute': 'G-flat major'}\r\n\r\n```\r\n\r\nHaving `attribute-list` in two locations working differently can't be changed now. Having `attribute-dicts` in two locations working differently isn't better and we don't have to."}}