Skip to content

Added ItemSearch.url_with_parameters #304

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

Merged
merged 6 commits into from
Sep 1, 2022
Merged

Conversation

wietzesuijker
Copy link
Contributor

Related Issue(s):

Description:

PR Checklist:

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

@gadomski gadomski linked an issue Aug 27, 2022 that may be closed by this pull request
@gadomski gadomski self-requested a review August 27, 2022 15:26
@gadomski gadomski added this to the 0.6.0 milestone Aug 30, 2022
@codecov-commenter
Copy link

Codecov Report

Base: 84.90% // Head: 84.98% // Increases project coverage by +0.08% 🎉

Coverage data is based on head (1950d8f) compared to base (c7fa924).
Patch coverage: 96.00% of modified lines in pull request are covered.

❗ Current head 1950d8f differs from pull request most recent head b7791a9. Consider uploading reports for the commit b7791a9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #304      +/-   ##
==========================================
+ Coverage   84.90%   84.98%   +0.08%     
==========================================
  Files          11       11              
  Lines         775      786      +11     
==========================================
+ Hits          658      668      +10     
- Misses        117      118       +1     
Impacted Files Coverage Δ
pystac_client/item_search.py 92.77% <96.00%> (-0.07%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

@wietzesuijker I've made a couple of changes to your branch:

  1. Fixed some typing issues
  2. I use requests to build the URL instead of doing it ourselves

I've force-pushed these changes, so you'll have to do a hard reset of your branch if you want to make any additional changes.

@scottyhq would you mind looking this over to make sure this is what you want?

I'm going to add an additional reviewer since I made some non-insignificant changes to the PR.

@gadomski gadomski requested a review from TomAugspurger August 30, 2022 21:20
Copy link
Collaborator

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

Looks good, but it'd be great to include a docstring. I believe it'll automatically show up in the api.rst.

@gadomski gadomski merged commit 2522dc1 into stac-utils:main Sep 1, 2022
@gadomski gadomski modified the milestones: 0.6.0, 0.5.2 Dec 12, 2022
@gadomski gadomski modified the milestones: 0.5.2, 0.6.0 Jan 27, 2023
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.

Retrieve formatted search url
4 participants