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

Preserve Collection assets on clone #834

Conversation

duckontheweb
Copy link
Contributor

@duckontheweb duckontheweb commented Jun 28, 2022

Related Issue(s):

Description:

Adds an assets argument to Item.__init__ and Collection.__init__ that accepts a dictionary mapping strings to Asset objects, which allows us to attach Assets to these objects as part of instantiation.

Also preserves Collection assets when using Collection.clone and adds regression test.

cc: @cholden-ag

PR Checklist:

  • Code is formatted (run pre-commit run --all-files)
  • 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.

@duckontheweb duckontheweb added bug Things which are broken enhancement labels Jun 28, 2022
@duckontheweb duckontheweb added this to the 1.5.0 milestone Jun 28, 2022
@duckontheweb duckontheweb requested a review from gadomski June 28, 2022 15:59
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2022

Codecov Report

Merging #834 (a2febda) into main (0c6074a) will decrease coverage by 0.00%.
The diff coverage is 93.93%.

@@            Coverage Diff             @@
##             main     #834      +/-   ##
==========================================
- Coverage   94.34%   94.33%   -0.01%     
==========================================
  Files          83       83              
  Lines       11923    11939      +16     
  Branches     1390     1400      +10     
==========================================
+ Hits        11249    11263      +14     
  Misses        495      495              
- Partials      179      181       +2     
Impacted Files Coverage Δ
tests/test_item.py 99.54% <91.66%> (-0.46%) ⬇️
tests/test_collection.py 99.56% <92.30%> (-0.44%) ⬇️
pystac/collection.py 92.46% <100.00%> (ø)
pystac/item.py 88.77% <100.00%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c6074a...a2febda. Read the comment docs.

@duckontheweb duckontheweb changed the title Fix/747 collection clone removes assets Preserve Collection assets on clone Jun 28, 2022
@cholden-ag
Copy link

Sorry to leave the PR to my issue hanging @duckontheweb and thanks for adding the fix. I think this looks good for fixing the clone() behavior on Collections, and having assets in the constructor will also improve the user experience for creating some Items/Collections since it'll take care of running add_asset

@duckontheweb duckontheweb requested a review from gadomski June 29, 2022 16:32
@duckontheweb duckontheweb merged commit b7fbd6e into stac-utils:main Jun 29, 2022
@duckontheweb duckontheweb deleted the fix/747-collection-clone-removes-assets branch June 29, 2022 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things which are broken enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collection assets removed on clone()
4 participants