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

Collection assets removed on clone() #747

Closed
cholden-ag opened this issue Feb 9, 2022 · 2 comments · Fixed by #834
Closed

Collection assets removed on clone() #747

cholden-ag opened this issue Feb 9, 2022 · 2 comments · Fixed by #834
Assignees
Labels
bug Things which are broken
Milestone

Comments

@cholden-ag
Copy link

Hello! First thanks for this wonderful library, it's been super helpful

I might be misunderstanding, but I think there is an issue with Collections losing their assets when you run .clone() because the attribute is not carried over when constructing a new instance. I saw some discussion about placing asset info in the Collection.extra_fields attribute (ref #293) but my understanding is that Collections now support having assets (e.g., there's a Collection.add_asset)

It looks to me like the asset attribute needs to be included in this call,
https://github.com/stac-utils/pystac/blob/main/pystac/collection.py#L556-L568
but was maybe omitted as the STAC spec evolved.

Example,

>>> import pystac
>>> coln = pystac.read_file("https://planetarycomputer.microsoft.com/api/stac/v1/collections/landsat-8-c2-l2")
# nice thumbnail!
>>> coln.assets
{'thumbnail': <Asset href=https://ai4edatasetspublicassets.blob.core.windows.net/assets/pc_thumbnails/landsat.png>}
# works
>>> coln.to_dict()['assets']
{'thumbnail': {'href': 'https://ai4edatasetspublicassets.blob.core.windows.net/assets/pc_thumbnails/landsat.png', 'type': 'image/png', 'title': 'Landsat 8 C2', 'roles': ['thumbnail']}}
# broken
>>> coln.clone().assets
{}

If this makes sense I'm happy to add a PR with a fix and tests

@duckontheweb
Copy link
Contributor

@cholden-ag Thanks for the detailed bug report, and for tracking down the issues. I would happily review a PR with tests!

In Item.clone we handle transferring assets by calling item.add_asset(...) after the new Item is created, so I think we should follow the same pattern for Collection.clone.

Alternatively, I would support adding an assets argument to both Item.__init__ and Collection.__init__. I'm not sure why we don't include that as an optional argument already, but it seems like it would be nice to be able to create Items and Collections with assets in a single line.

@duckontheweb duckontheweb added the bug Things which are broken label Feb 16, 2022
@duckontheweb duckontheweb added this to the 1.5.0 milestone May 12, 2022
@duckontheweb
Copy link
Contributor

@cholden-ag Do you have bandwidth to put in a PR for this issue in the near future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things which are broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants