Make ExtensionManagementMixin.add_to idempotent

[duckontheweb]

Related Issue(s): #

• Closes ExtensionManagementMixin.add_to should be idempotent #369

Description:

pystac.extensions.base.ExtensionManagementMixin.add_to will only add the schema URI for the inheriting extension class if it is not already in the object's stac_extensions list.

I looked into making stac_extensions a set instead of a list, but the extra complexity to ensure that the set was properly serialized and deserialized did not seem worth the effort. If that seems like the better approach, I can explore it in more depth, though.

PR Checklist:

• Code is formatted (run scripts/format)
• Tests pass (run scripts/test)
• 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.

[gadomski]

👍🏽 to keepin' it simple for now, and if we identify this as a performance bottleneck later we know how to proceed (seems unlikely).

[lossyrob]

We also talked about checking that the object to see if it has the extension on FooExtension.ext(...), and throwing if it didn't exist, and also adding a add_if_missing flag to that method to allow a one-liner to add the extension and get the extension object. Is that still a plan? If so, should that be tackled in another PR or added to this?

[duckontheweb]

We also talked about checking that the object to see if it has the extension on FooExtension.ext(...), and throwing if it didn't exist, and also adding a add_if_missing flag to that method to allow a one-liner to add the extension and get the extension object. Is that still a plan? If so, should that be tackled in another PR or added to this?

I started to tackle that as part of this PR, but there's a wrinkle with how to handle that when extending an Asset with no owner, since it won't have any extension info associated with it. I figured I would work through that in comments on #370 and merge this in the meantime, but I'm also fine with keeping this open until that's resolved.

[duckontheweb]

I figured I would work through that in comments on #370 and merge this in the meantime, but I'm also fine with keeping this open until that's resolved.

I will actually just implement that as part of this PR and we can get feedback on the approach that way. Probably easier than trying to discuss it in an issue.

[duckontheweb]

@lossyrob Sorry to keep waffling on this, but I think it would be best to merge this and tackle #370 in a separate PR. There are some challenges around how and where to implement that exception and how to handle Asset instances without an owner that might be best to work out separately, and it probably doesn't make sense to hold up this change in the meantime.

[codecov-commenter]

Codecov Report

Merging #419 (77b5d97) into main (e7d4bdc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #419   +/-   ##
=======================================
  Coverage   90.67%   90.67%           
=======================================
  Files          40       40           
  Lines        5105     5106    +1     
=======================================
+ Hits         4629     4630    +1     
  Misses        476      476           

Impacted Files	Coverage Δ
pystac/extensions/base.py	91.66% <100.00%> (+0.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7d4bdc...77b5d97. Read the comment docs.