Support for more collection types

[hashhar]

I'm working on adding support for more collection types to enable implementing https://github.com/beetbox/beets/issues/3549.

I had a few questions around how should I do this? I see clear opportunity to DRY some things up to reduce code duplication (since all types of collections use the same API) but that would break existing applications which are using the older methods.

One thing I can do is to leave the older methods there but change their implementation to use the more generic methods I wish to add so that both new and old users can co-exist.

What are other people's thoughts?

[hashhar]

FYI, my work can be followed at https://github.com/hashhar/python-musicbrainzngs/tree/extended-collections but there won't be anything to see there until someone (@alastair ?) can let me know which way to go.

As a last resort I may submit a PR with what I think is good and can rework it according to whatever feedback I get.

[hashhar]

@alastair I think I'm going with the approach used in the _do_collection_query and the methods that use it. It'll allow using both named functions and an easy way to add new types of collections.

[alastair]

Thanks for the contributions. I'll take a look at this in the coming week!

What were your ideas for a less-duplicated version of this code? Do you have an idea about what the interface might look like?

[hashhar]

I think my idea would've made the interface a little difficult to use. I was thinking of exposing the _do_collection_put and _do_collection_delete methods as public ones and not add the specific functions which I've added (add_areas_to_collection, add_artists_to_collection etc.).

The users of the package could then directly pass in the correct collection_type to the _do_collection_put/remove methods. But that would increase chances of mistakes and would need people to check what all types were available.

In a magical world I'd expect to be able to have the add_type_to_collection and delete_type_from_collection methods be autogenerated for all the possible types.

I'm satisfied with what I've done for now - just need to test this a some more (mainly around release-groups).

[hashhar]

@alastair just wanted to remind you of this PR.

I've tested this with all the new endpoints that I've added and it's working.

One concern I had was that right now I'm throwing exceptions when a list longer than 400 ids is passed in which might break existing users. I was thinking to add a loop inside the function to break into chunks of 400 instead of failing the request altogether. Will make the API easier for the clients to consume - with an increased chance of running into rate limiting.

[alastair]

I've not forgotten this! I'm trying to find some time in the weekends to finish looking at this, but I keep running out of time. The PR looks good in general, so I'll try and get it merged as soon as possible

[hashhar]

@alastair Just wanted to check if you had a chance to look at this.

[alastair]

Thanks for your friendly reminders. It's still been quite busy here, but this is on my pymb list.

[bartkl]

I'm yearning for this as well. Would be great if this could get merged :). Thanks for the great work!