Extensions are not required on catalogs #127
Conversation
|
Everywhere else we ask for extensions, we decode them as an optional list of strings, so this one was just an oversight 🤷♂️ |
| @@ -58,7 +58,7 @@ object StacCatalog { | |||
| ) => | |||
| StacCatalog.apply( | |||
| version, | |||
| extensions, | |||
| extensions getOrElse Nil, | |||
There was a problem hiding this comment.
This limits the set of changes here to only the encoders/decoders - which is nice because it reduces the downstream dependencies. But one thing that I think happens as a result is if stac4s reads a catalog without stac_extensions it will always write the same object with an empty list.
Should we change the encoder to behave the same as the decoder so it's consistent?
There was a problem hiding this comment.
I don't think so, but I also don't think that fields should be both optional and possibly empty lists 🤷♂️.
I can't think of a way that encoding an empty list will hurt anything. It might be annoying if you keep firing PATCHes at Franklin and despite your best efforts you just can't get rid of the stac_extensions field. I don't know.
There was a problem hiding this comment.
yeah I agree - I wish it was just an empty list too. The approach here makes sense to me.
Overview
This PR makes
stac_extensionsoptional when decoding catalogs.Checklist
Testing Instructions
stac_extensionswhen decoding, we ask for anOption[List[String]]instead of aList[String]Closes #121