Fix #186 and related issues#221
Merged
Merged
Conversation
The logic around determining when to attempt to merge collection properties into items was broken. This should only happen for STAC 0.9.0 if the Item implements the 'commons' extension. The logic was checking for that extension in an existing list, but if the JSON did not have a 'stac_extensions' property it was performing the merge. This changes it to avoid the common property merge logic if no stac_extensions property exists.
The logic in the common properties merging functionality expects a collection (or a dict representing collection JSON) be cached according to the ID associated with the collection property in an Item. If something else is cached there (for example, if a collection was read in as a Catalog) this produced a confusing error. This changes the logic to throw a ValueError in that case.
This commit changes the behavior of the from_dict methods of Catalog and Collection to pass in the href to the constructor so that caching logic utilizes the HREF correctly. This change will change behavior in that a catalog or collection that is read in with a HREF that is different then the stated self HREF on the object would previously use the HREF in the link. The to_dict methods will now set the given HREF as the self link if provided, even if there's an existing, differing self link. This should only affect cases when there are incorrect self links in STAC objects.
This change was missed in #214. In that PR, the set_self_href logic that existed on Collection was promoted to be the logic of all set_self_hrefs on STACObjects. This makes this method unnecessary, and causes duplicate calls to cache logic.
Codecov Report
@@ Coverage Diff @@
## develop #221 +/- ##
===========================================
- Coverage 92.62% 92.59% -0.03%
===========================================
Files 28 28
Lines 3375 3364 -11
===========================================
- Hits 3126 3115 -11
Misses 249 249
Continue to review full report at Codecov.
|
Member
Author
|
Code coverage decreases because code removal; all additions tested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes issues uncovered while investigating #186.
stac_extensionswas not declared in the Item.The two other changes below relate to the caching changes made in #214:
from_dictwas causing root objects to be cached twice - once with it's ID key and once when the HREF was set on the object. This changes the logic to ensure that if an object is read in with an HREF, it's cached only by its HREF.set_self_hrefon Collection was promoted to be the implementation for all STACObjects. This method on Collection should have been deleted in Refactor caching to utilize HREFs and parent IDs #214, but is done so here.Fixes #186