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

Allow nullable stac_extensions #1109

Merged
merged 1 commit into from
May 4, 2023
Merged

Allow nullable stac_extensions #1109

merged 1 commit into from
May 4, 2023

Conversation

jsignell
Copy link
Member

@jsignell jsignell commented May 1, 2023

Description: I noticed recently while working on VEDA (https://staging-stac.delta-backend.com/) that if stac_extensions is set to null, with #1091 the collection can't be opened so as not to break people who might have have this mild invalidity in their catalogs.

NOTE: I none of these collections with stac_extensions set to null fail validation because by the time you have created a collection from a dict and then dumped it back to a dict null has been stripped out.

Example:

from pystac import Catalog

catalog = Catalog.from_file('https://staging-stac.delta-backend.com/')
failed = []
for c in catalog.get_collections():
    try:
        c.validate()
    except:
        failed.append(c)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[1], line 5
      3 catalog = Catalog.from_file('https://staging-stac.delta-backend.com/')
      4 failed = []
----> 5 for c in catalog.get_collections():
      6     try:
      7         c.validate()

File ~/pystac/pystac/stac_object.py:379, in STACObject.get_stac_objects(self, rel, typ, modify_links)
    377 link = links[i]
    378 if link.rel == rel:
--> 379     link.resolve_stac_object(root=self.get_root())
    380     if typ is None or isinstance(link.target, typ):
    381         yield cast(STACObject, link.target)

File ~/pystac/pystac/link.py:322, in Link.resolve_stac_object(self, root)
    319     if stac_io is None:
    320         stac_io = pystac.StacIO.default()
--> 322 obj = stac_io.read_stac_object(target_href, root=root)
    323 obj.set_self_href(target_href)
    324 if root is not None:

File ~/pystac/pystac/stac_io.py:236, in StacIO.read_stac_object(self, source, root, *args, **kwargs)
    216 """Read a STACObject from a JSON file at the given source.
    217 
    218 See :func:`StacIO.read_text <pystac.StacIO.read_text>` for usage of
   (...)
    233     contained in the file at the given uri.
    234 """
    235 d = self.read_json(source, *args, **kwargs)
--> 236 return self.stac_object_from_dict(
    237     d, href=source, root=root, preserve_dict=False
    238 )

File ~/pystac/pystac/stac_io.py:168, in StacIO.stac_object_from_dict(self, d, href, root, preserve_dict)
    163     merge_common_properties(
    164         d, json_href=href_str, collection_cache=collection_cache
    165     )
    167 info = identify_stac_object(d)
--> 168 d = migrate_to_latest(d, info)
    170 if info.object_type == pystac.STACObjectType.CATALOG:
    171     result = pystac.Catalog.from_dict(
    172         d, href=href_str, root=root, migrate=False, preserve_dict=preserve_dict
    173     )

File ~/pystac/pystac/serialization/migrate.py:198, in migrate_to_latest(json_dict, info)
    195         result["stac_extensions"] = []
    197 pystac.EXTENSION_HOOKS.migrate(result, version, info)
--> 198 for ext in result["stac_extensions"][:]:
    199     if ext in removed_extension_migrations:
    200         object_types, migration_fn = removed_extension_migrations[ext]

TypeError: 'NoneType' object is not subscriptable

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.

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.

Change seems fine, but this could use a unit test? Also needs CHANGELOG entry.

@jsignell jsignell requested a review from gadomski May 4, 2023 18:46
@gadomski gadomski enabled auto-merge May 4, 2023 18:47
@gadomski gadomski disabled auto-merge May 4, 2023 18:48
@gadomski gadomski enabled auto-merge May 4, 2023 18:48
@gadomski gadomski added this to the 1.8 milestone May 4, 2023
@gadomski gadomski added this pull request to the merge queue May 4, 2023
Merged via the queue into main with commit 70efbb6 May 4, 2023
@gadomski gadomski deleted the stac_extensions branch May 4, 2023 18:59
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.

2 participants