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

Make sure that get_items is backwards compatible #1139

Merged
merged 5 commits into from
Jun 2, 2023

Conversation

jsignell
Copy link
Member

@jsignell jsignell commented Jun 1, 2023

Related Issue(s):

Description:

I didn't quite go so far as reverting, but I did make the change backwards compatible by not relying on the new kwarg internally. I ran the pystac-client tests locally and the one that was failing in stac-utils/pystac-client#485 passes now.

Note: I didn't add tests.There should be only 2 lines that aren't tested (the ones in the TypeError blocks), so it seemed like more trouble than it was worth, but just let me know if you think they are necessary.

PR Checklist:

  • pre-commit hooks pass locally
  • Tests pass (run scripts/test)
  • Documentation has been updated to reflect changes, if applicable
  • This PR maintains or improves overall codebase code coverage.
  • Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

@jsignell jsignell self-assigned this Jun 1, 2023
@jsignell jsignell requested a review from gadomski June 1, 2023 14:59
@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (d55cc15) 90.92% compared to head (e07cdc5) 90.93%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1139      +/-   ##
==========================================
+ Coverage   90.92%   90.93%   +0.01%     
==========================================
  Files          49       49              
  Lines        6631     6642      +11     
  Branches      975      981       +6     
==========================================
+ Hits         6029     6040      +11     
  Misses        422      422              
  Partials      180      180              
Impacted Files Coverage Δ
pystac/catalog.py 93.42% <100.00%> (+0.09%) ⬆️
pystac/collection.py 95.48% <100.00%> (+0.08%) ⬆️

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

@jsignell
Copy link
Member Author

jsignell commented Jun 1, 2023

I'm taking a look at the tests to get the coverage back up. I think I probably took a lot of get_item testing out in my original PR.

@jsignell
Copy link
Member Author

jsignell commented Jun 1, 2023

phew ok, hopefully that'll do it

pystac/catalog.py Outdated Show resolved Hide resolved
pystac/collection.py Outdated Show resolved Hide resolved
tests/test_catalog.py Outdated Show resolved Hide resolved
@jsignell jsignell requested a review from gadomski June 2, 2023 14:10
@jsignell jsignell added this pull request to the merge queue Jun 2, 2023
Merged via the queue into stac-utils:main with commit 25ed95c Jun 2, 2023
@jsignell jsignell deleted the backwards-compat-recursive branch June 2, 2023 19:28
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.

Revert breaking changes to get_items
2 participants