Skip to content

Conversation

@gadomski
Copy link
Member

@gadomski gadomski commented Jun 3, 2022

Related Issue(s):

Description: After digging a bit, I think it does not make sense to allow Client.open to work for collections, since we have ClientCollection to handle that case. This PR improves the error message when trying to open a collection with Client.open.

Now:

$ stac-client search https://planetarycomputer.microsoft.com/api/stac/v1/collections/sentinel-2-l2a
Could not open Client (href=https://planetarycomputer.microsoft.com/api/stac/v1/collections/sentinel-2-l2a), expected type=Catalog, found type=Collection

Before:

$ stac-client search https://planetarycomputer.microsoft.com/api/stac/v1/collections/sentinel-2-l2a
{'id': 'sentinel-2-l2a', 'type': 'Collection', <snip>...</snip> L2A (bottom-of-atmosphere).'} does not represent a Client instance

PR Checklist:

  • Code is formatted
  • Tests pass
  • Changes are added to the CHANGELOG

When trying to open a collection, we present a better exception / message.
@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2022

Codecov Report

Merging #222 (7f4b7fb) into main (e0166d3) will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #222      +/-   ##
==========================================
+ Coverage   83.75%   83.96%   +0.21%     
==========================================
  Files           9       10       +1     
  Lines         677      686       +9     
==========================================
+ Hits          567      576       +9     
  Misses        110      110              
Impacted Files Coverage Δ
pystac_client/client.py 81.94% <100.00%> (+1.94%) ⬆️
pystac_client/errors.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0166d3...7f4b7fb. Read the comment docs.

@philvarner
Copy link
Collaborator

One other related issue is that we have a bunch of methods that have tying that they return ClientCollection, but if a StacIO instance that's not a StacApiIO is used, they actually just return a Collection. I'm not sure what expectations from a user would be in these cases.

@gadomski
Copy link
Member Author

gadomski commented Jun 3, 2022

One other related issue is that we have a bunch of methods that have tying that they return ClientCollection, but if a StacIO instance that's not a StacApiIO is used, they actually just return a Collection. I'm not sure what expectations from a user would be in these cases.

My initial instinct is that it should error -- changing the type of a method's return value based on the type of a member feels very funny. But I don't have a full sense of the implications of that change, or the cases where someone would be returning a ClientCollection.

@gadomski gadomski merged commit d1b12e3 into main Jun 3, 2022
@gadomski gadomski deleted the issues/103-client-open-collection branch June 3, 2022 18:38
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.

Client.open and collections

4 participants