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

RFC: Add move/copy/delete operations for local assets #911

Closed
gadomski opened this issue Nov 7, 2022 · 3 comments · Fixed by #1158
Closed

RFC: Add move/copy/delete operations for local assets #911

gadomski opened this issue Nov 7, 2022 · 3 comments · Fixed by #1158
Assignees
Labels
discussion An issue to capture a discussion enhancement
Milestone

Comments

@gadomski
Copy link
Member

gadomski commented Nov 7, 2022

Summary

Add methods to Asset and Item to support moving/copying/deleting assets and their referenced data files. Prompted by #61

Motivation

Currently, asset move/copy operations are handled by stactools (e.g. https://stactools.readthedocs.io/en/stable/api.html#module-stactools.core.copy). For local filesystem use-cases, there's no reason why PySTAC couldn't support filesystem operations on data files; no extra dependencies would be required.

Explanation

Assets would get two new methods, move and copy:

class Asset:
    def move(self, href: str) -> Asset:
        """Moves this asset's file to a new location on the local filesystem, setting the asset href accordingly.

        Modifies the asset in place, and returns the same asset.
        """
        ...

    def copy(self, href: str) -> Asset:
        """Copies this asset's file to a new location on the local filesystem, setting the asset href accordingly.

        Modifies the asset in place, and returns the same asset.
        """
        ...

Item would get one new method, delete_asset:

class Item:
    def delete_asset(self, key: str):
        """Deletes the asset at the given key, and removes the asset's data file from the local filesystem.
        
        It is an error to attempt to delete an asset's file if it is on a remote filesystem.
        To delete the asset without removing the file, use `del item.assets["key"]`.
        """
        ...

Drawbacks

This adds code and API complexity, and blurs the separation of responsibilities between stactools and pystac.

Unresolved questions

Future possibilities

  • If we eventually relax the "don't have lots of dependencies" requirement for PySTAC, these asset copy/move/delete operations could/would become more powerful and useful.
@gadomski gadomski added enhancement discussion An issue to capture a discussion labels Nov 10, 2022
@gadomski gadomski added this to the 2.0 milestone Jan 31, 2023
@gadomski gadomski modified the milestones: 2.0, 1.8 Mar 24, 2023
@jsignell
Copy link
Member

I was thinking about picking up this item and was originally thinking: "why isn't this just clone+save" but then I realized that you are talking about moving/saving the data referenced by the asset, not the json that the asset is composed of. I think this is a neat idea (and kind of analogous to something that I'd been thinking might be nice for assets which is a generic open method).

I am worried though that my confusion is indicative of something slightly odd in the naming here. How would you feel about adding another word so instead of move and copy it was move_data and copy_data?

@jsignell jsignell self-assigned this Jun 12, 2023
@gadomski
Copy link
Member Author

gadomski commented Jun 12, 2023

Agreed that "asset" is a confusing term, as it can be used to refer to the STAC Asset (the JSON) or the data file itself. IMO "move an asset" must mean that you're moving the file, since an Asset JSON cannot stand on its own (and therefore cannot be moved). But I probably am too deep into STAC to have a good, unbiased perspective.

move_data and copy_data seem fine, but I would like to get a third perspective to make sure we're doing the least surprising thing possible.

@jsignell
Copy link
Member

Yeah honestly only in starting on this did I remember that an asset metadata is just a piece of the item json and not its own file. I am feeling fine about move and copy now. I have delete too. I thought it might be good for people who want to override these methods in a custom asset class. You can see what you think.

@jsignell jsignell linked a pull request Jun 13, 2023 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion An issue to capture a discussion enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants