rename id in connection.load_collection

[jonathom]

Should the parameter also be changed in datacube.load_collection() and imagecollectionclient.load_collection()?

[m-mohr]

Should this be deprecated instead of being removed?

[jonathom]

Would I just put @deprecated("Use 'id' instead") above the :param collection_id line?

[m-mohr]

Sounds good to me, but @soxofaan should have the final vote.

[soxofaan]

I don't think we can make a breaking change like this.
Existing code that uses keyword argument load_collection(collection_id="S2") will break.

There should be a fallback mechanism:

• give id default value None
• add a **kwargs and check if collection_id is set (and raise deprecation warning if that is the case)

[m-mohr]

Yes, that's what I meant to say :-D

[jonathom]

@soxofaan

def load_collection(
    self,
    id: str = None,
[...]
    if "collection_id" in kwargs:
        id = kwargs["collection_id"]
        warnings.warn("The use of `collection_id` is deprecated, use `id` instead.", DeprecationWarning)

like so?

[soxofaan]

I think the load_collection methods of DataCube and ImageCollectionClient should also be addressed

but here it's fine to do it in a backwards incompatible way because these are practically "internal" methods nobody should be using

[soxofaan]

also to do:

• test coverage for old style and new style
• Changelog entry because this is somewhat "breaking" (with fallback mechanism)

[jonathom]

test coverage for old style and new style

What do you mean by this?

[soxofaan]

test that both load_collection(collection_id="Sentinel2") and load_collection(id="Sentinel2") work

[jonathom]

Alright, did that. Didn't add tests for imagecollectionclient and datacube yet, but I can if you want @soxofaan.