-
Notifications
You must be signed in to change notification settings - Fork 48
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
Propagate root StacAPIIO to ItemSearch in Client.get_items #126
Propagate root StacAPIIO to ItemSearch in Client.get_items #126
Conversation
Codecov Report
@@ Coverage Diff @@
## main #126 +/- ##
=======================================
Coverage 79.59% 79.59%
=======================================
Files 9 9
Lines 549 549
=======================================
Hits 437 437
Misses 112 112
Continue to review full report at Codecov.
|
@@ -23,7 +23,7 @@ def get_items(self) -> Iterable["Item_Type"]: | |||
|
|||
link = self.get_single_link('items') | |||
if link is not None: | |||
search = ItemSearch(link.href, method='GET') | |||
search = ItemSearch(link.href, method='GET', stac_io=self.get_root()._stac_io) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing technically wrong here, but accessing a "private" member of an object retrieved via a function call feels like we're breaking abstraction. This isn't surprising given the nature of how the StacIO
stuff has to work, but I'd just flag this for a bit of code smell w/ maybe the chance to refactor sometime down the line. No changes required now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, thanks for flagging that. I had originally added a "public" stac_io
property to pystac.Catalog
in stac-utils/pystac#590, but we ultimately decided that it wasn't needed as part of that PR, so I took it back out. Given that there are a couple of places where it is accessed in the pystac-client
code, it might be worth revisiting that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened stac-utils/pystac#675 to discuss adding a "public" attribute/property.
Related Issue(s):
Description:
Ensures that query string parameters and headers given in
Client.open
orClient.from_file
are used in requests fromCollectionClient
instances fetched from that API.PR Checklist: