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.

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

Fix to logic determining if context.matched in response #646

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

dchandan
Copy link
Contributor

Description:
Currently, if an API does not return the context.matched key in the search response then ItemSearch.matched fails with a KeyError. This is not ideal, because if for any reason appropriate information needed to determine the number of matched results is not present in the response, then according to the function's description, the function should return None. This PR makes a minor fix to ameliorate the issue.

P.S. I realize now that I've misspelled "Matched" as "Marched" in my branch name. Sorry.

PR Checklist:

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

@dchandan
Copy link
Contributor Author

Okay, I just noticed that my editor did some black formatting as well. I hope this is OK and there are no rule violations with the performed formatting.

CHANGELOG.md Outdated Show resolved Hide resolved
@gadomski
Copy link
Member

@dchandan yeah, the formatting broke CI. Please use pre-commit to ensure the hooks run correctly: https://pystac-client.readthedocs.io/en/stable/contributing.html#code-quality-checks.

Otherwise, looks good, thank you!

@dchandan
Copy link
Contributor Author

@dchandan yeah, the formatting broke CI. Please use pre-commit to ensure the hooks run correctly: https://pystac-client.readthedocs.io/en/stable/contributing.html#code-quality-checks.

Otherwise, looks good, thank you!

Thanks for the clarification @gadomski. pre-commit issue should be fixed now.

@gadomski gadomski enabled auto-merge (squash) February 28, 2024 13:24
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.44%. Comparing base (21435b0) to head (0a06861).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #646   +/-   ##
=======================================
  Coverage   93.43%   93.44%           
=======================================
  Files          13       13           
  Lines         990      991    +1     
=======================================
+ Hits          925      926    +1     
  Misses         65       65           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gadomski gadomski merged commit 3c02e58 into stac-utils:main Feb 28, 2024
18 checks passed
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.

None yet

3 participants