Allow to save a catalog at a specific location #565
Conversation
Codecov Report
@@ Coverage Diff @@
## main #565 +/- ##
==========================================
+ Coverage 94.35% 94.68% +0.33%
==========================================
Files 71 75 +4
Lines 10409 10758 +349
Branches 1089 1052 -37
==========================================
+ Hits 9821 10186 +365
+ Misses 419 396 -23
- Partials 169 176 +7
Continue to review full report at Codecov.
|
|
I think this will also help with #425 |
pystac/catalog.py
Outdated
| This is olnly used if the catalog type is | ||
| ``CatalogType.ABSOLUTE_PUBLISHED``. |
There was a problem hiding this comment.
Minor typo here:
| This is olnly used if the catalog type is | |
| ``CatalogType.ABSOLUTE_PUBLISHED``. | |
| This is only used if the catalog type is | |
| ``CatalogType.ABSOLUTE_PUBLISHED``. |
It might also be good to add a check at the beginning of this method that raises a STACError if dest_href is not None and root.catalog_type != CatalogType.ABSOLUTE_PUBLISHED. I could see users being confused if they pass in a dest_href that just gets ignored silently.
There was a problem hiding this comment.
I restricted it to CatalogType.ABSOLUTE_PUBLISHED, because the original issue in the stactools repo mentioned the functionality for that case, but it's true that here it makes sense for other types as well (If anyone has a different opinion here or I am missing something, please comment here). I have removed that check and now the dest_href parameter is used regardless of the catalog type
There was a problem hiding this comment.
Thanks for tackling this @volaya! This approach looks good to me, but I'm wondering if we really need to restrict dest_href to only apply to "absolute published" catalogs. I'm admittedly not as familiar with the nuances of the different catalog types, but it seems like if someone has to explicitly pass in the dest_href argument then they may have a good reason to do it for other catalog types. Maybe that would require much more complicated updating of the catalog links, though, not sure 🤷🏻
pystac/catalog.py
Outdated
| include_self_link=items_include_self_link | ||
| ) | ||
| item = cast(pystac.Item, item_link.target) | ||
| item_dest_href = None |
There was a problem hiding this comment.
| item_dest_href = None |
This ends up just being unused
There was a problem hiding this comment.
This looks good to me @volaya, thanks for contributing this!
You mentioned in the original description that this is a WIP. Is that still true or is it ready to merge?
|
@duckontheweb This should be now fine to merge. I added the matching functionality in StacTools here: stac-utils/stactools#175 |
|
Coverage upload seems to be failing due to codecov/codecov-action#437, but everything else passes, so I'll merge this. |
This PR allows to specify a folder where a catalog should be saved, to be used instead of the pth taken for the catalog self href
This will allow implementing the functionaity requested at stac-utils/stactools#12
It is still a work in progress. I am creating this PR to share the approach and gather some feedback about it. I will add unit tests and implement the corresponding code in StacTools later.