stac-utils / pystac

Python library for working with any SpatioTemporal Asset Catalog (STAC)
https://pystac.readthedocs.io
Other
358 stars 120 forks source link

Docs on adding extensions is misleading or incomplete #602

Closed alexgleith closed 2 years ago

alexgleith commented 3 years ago

The Extensions docs say that to add an extension you do:

import pystac
from pystac.extensions.eo import EOExtension

item = Item(...)  # See docs for creating an Item
eo_ext = EOExtension.ext(item)

But in practice you need to do:

EOExtension.add_to(item)
eo = EOExtension.ext(item)

Is this intended, and if yes, can we tweak the docs please?

duckontheweb commented 3 years ago

Thank for raising this @alexgleith.

The behavior of raising an exception if the Item does not already contain the schema URI is intended (see this PR comment in stactools for some background).

Those docs assumed that the Item already has the EO extension schema URI in its stac_extensions list, but we should definitely make it more clear that you need to use either EOExtension.add_to(item) or eo_ext = EOExtension.ext(item, add_if_missing=True) to add the extensions to an Item that does not already implement it.

alexgleith commented 3 years ago

Is there a good reason why that doesn't default to true?

gadomski commented 3 years ago

https://github.com/stac-utils/stactools/pull/113#discussion_r639860502 has some discussion about why it's not automatic, but basically we wanted to force folks to be explicit about adding any new extension schema urls so we didn't, e.g., mutate the object in question (by adding an extension schema url) when a person was treating the object as "read-only".

alexgleith commented 3 years ago

Hmm, I can see the logic here for when dealing with an existing item.

I reckon the docs should cover it better still. Maybe I need to read them again to see if I missed it somewhere!

duckontheweb commented 2 years ago

@alexgleith I opened #701 to try to clarify this better in the Concepts docs. Let me know if you think that covers it.

duckontheweb commented 2 years ago

Closed via #701