-
Notifications
You must be signed in to change notification settings - Fork 121
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
New pickling methods loads all absolute links #1336
Comments
I'm not sure this is true, see follow-on comment for more info. |
@emmanuelmathot can you provide a minimum-reproducible example so I can be sure I'm testing against the same problem? I was not able to reproduce the behavior you described with this test: def test_non_existent_link_during_deepcopy(item: Item) -> None:
item.add_link(pystac.Link("non-existent-asset", "../not-a-dir/not-a-file"))
item = copy.deepcopy(item)
assert item.get_single_link("non-existent-asset").href == "../not-a-dir/not-a-file" |
sure, please find the test in this branch: https://github.com/emmanuelmathot/pystac/blob/pickle/tests/test_item.py#L686 |
@emmanuelmathot do you have an example that includes creating that test file? I'd like to be able to dig into the process that's actually doing the href modifications. |
No I do not but a very simple item with one link with absolute s3 href makes the error. This is really similar to what you mentioned here
when I put d["links"] = [
link.to_dict(transform_href=False) if link.get_href(transform_href=False) else link for link in d["links"]
] There is no more error |
@emmanuelmathot got it, thanks. Fix in #1337 which we'll release after merging. |
Since the PR #1285 and more specifically this change, it seems that a deepcopy of an item will load all the links with an absolute href.
Is it intended or am I missing something?
This causes issue when loading assets using
get_assets
method that makes first a deep copy of the stac object that uses pickling.When I load an item with unreachable links (e.g. s3 url but no custom IO reader set) and try to list the assets, it raises an issue.
The text was updated successfully, but these errors were encountered: