Skip to content

Expand get_items - deprecate get_all_items and Catalog.get_item #1075

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 11 commits into from
Apr 3, 2023

Conversation

jsignell
Copy link
Member

@jsignell jsignell commented Mar 31, 2023

Related Issue(s):

Description:

Instead of using get_all_items people should use get_items(recursive=True) and instead of Catalog.get_item(id) people should use next(Catalog.get_items(id), None)

TO DO:

  • make tests fail on deprecation warnings
  • refactor code to not use deprecated methods
  • add new tests
  • explicitly expect warnings in specific tests
  • refactor examples to not use deprecated methods

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.

@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2023

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.25%. Comparing base (25f2050) to head (a4e971f).
Report is 236 commits behind head on main.

Files Patch % Lines
pystac/summaries.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1075   +/-   ##
=======================================
  Coverage   90.25%   90.25%           
=======================================
  Files          47       47           
  Lines        6227     6230    +3     
  Branches      935      934    -1     
=======================================
+ Hits         5620     5623    +3     
  Misses        427      427           
  Partials      180      180           

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

@jsignell jsignell changed the title [WIP] Expand get_items - deprecate get_all_items and Catalog.get_item Expand get_items - deprecate get_all_items and Catalog.get_item Mar 31, 2023
@jsignell jsignell marked this pull request as ready for review March 31, 2023 21:04
@gadomski gadomski self-requested a review April 1, 2023 01:06
@gadomski gadomski added this to the 1.8 milestone Apr 1, 2023
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.

Just two minor notebook fixups, otherwise looks good to me. I'm going to ask for another set of eyes on this one, just because it's a non-insignificant API change we're committing too, and I want to make sure it makes sense to someone else.

Oh, needs a CHANGELOG as well.

@gadomski gadomski requested a review from TomAugspurger April 3, 2023 13:07
jsignell and others added 2 commits April 3, 2023 10:03
Co-authored-by: Pete Gadomski <pete.gadomski@gmail.com>
@jsignell jsignell requested a review from gadomski April 3, 2023 14:15
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.

Makes sense to me.

@gadomski gadomski added this pull request to the merge queue Apr 3, 2023
Merged via the queue into stac-utils:main with commit cb70734 Apr 3, 2023
@jsignell jsignell deleted the get-items branch April 4, 2023 13:26
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.

4 participants