Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Aggregation of queryables across select collections #511

Merged
merged 9 commits into from
May 11, 2023
Merged

Aggregation of queryables across select collections #511

merged 9 commits into from
May 11, 2023

Conversation

ircwaves
Copy link
Member

@ircwaves ircwaves commented May 5, 2023 •
edited
Loading

Related Issue(s):

Description:

PR Checklist:

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

@ircwaves ircwaves marked this pull request as draft May 5, 2023 19:56
@ircwaves
Copy link
Member Author

ircwaves commented May 5, 2023 •
edited
Loading

This Works™ but mypy's complaints confirm that I should shadow get_queryables in the pystac_client.client.Client class to properly separate concerns.

@gadomski gadomski linked an issue May 8, 2023 that may be closed by this pull request
Allow aggregation of queryables across collections #505
Closed
@gadomski gadomski self-requested a review May 8, 2023 12:29
gadomski
gadomski requested changes May 8, 2023
View reviewed changes
Copy link
Member

@gadomski gadomski left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment #505 (comment) has some answers to architecture questions, as well. I didn't look at the mypy issue too deeply yet, might wait until the next review request.

pystac_client/mixins.py Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
pystac_client/mixins.py Outdated Show resolved Hide resolved
@gadomski gadomski added this to the 0.7.0 milestone May 9, 2023
@ircwaves ircwaves force-pushed the union-queryables branch from 7398c03 to a973010 Compare May 10, 2023 19:16
ircwaves and others added 2 commits May 11, 2023 07:28
@ircwaves ircwaves force-pushed the union-queryables branch from a973010 to 36ded7f Compare May 11, 2023 11:29
ircwaves added 3 commits May 11, 2023 13:10
@ircwaves
ruff reordered imports
1fd2a3f
@ircwaves
update: get_collection no longer returns Optional
3e89be4 
Previously Client.get_collection returned None if the requested collection was
not found. Now it has been updated to raise an exception if the requested
collection does not exist.
@ircwaves
move Client-specific get_queryables implementation to Client class
550e186
@ircwaves ircwaves force-pushed the union-queryables branch from 36ded7f to 550e186 Compare May 11, 2023 17:30
@codecov-commenter
Copy link

codecov-commenter commented May 11, 2023 •
edited
Loading

Codecov Report

Patch coverage: 87.87% and project coverage change: -0.15 ⚠️

Comparison is base (b9e1696) 88.49% compared to head (a30f3e8) 88.34%.

❗ Current head a30f3e8 differs from pull request most recent head 428fadf. Consider uploading reports for the commit 428fadf to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
- Coverage   88.49%   88.34%   -0.15%     
==========================================
  Files          13       13              
  Lines         965      987      +22     
==========================================
+ Hits          854      872      +18     
- Misses        111      115       +4
Impacted Files Coverage Δ
pystac_client/client.py 90.60% <84.00%> (-1.65%) ⬇️
pystac_client/mixins.py 94.59% <100.00%> (+0.30%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ircwaves ircwaves marked this pull request as ready for review May 11, 2023 19:01
@ircwaves ircwaves requested a review from gadomski May 11, 2023 19:01
gadomski
gadomski approved these changes May 11, 2023
View reviewed changes
pystac_client/client.py Outdated Show resolved Hide resolved
@gadomski gadomski enabled auto-merge (squash) May 11, 2023 19:09
@gadomski gadomski merged commit f27f4dd into stac-utils:main May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gadomski gadomski gadomski approved these changes

Assignees

@ircwaves ircwaves

Labels
None yet
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.

Allow aggregation of queryables across collections
3 participants
@ircwaves @codecov-commenter @gadomski