Upgrades/stac #71
Upgrades/stac #71
Conversation
abradley60
commented
Jun 19, 2025
abradley60
commented
- Creating a valid stac metadata document
- tested with the stac-validator tool
- In code validation has been temporarily disables while test product are being created
sar_pipeline/aws/cli.py
Outdated
| burst_stac_manager.save(burst_folder / stac_filename) | ||
| # TODO validate the stac item when finalised | ||
| # burst_stac_manager.item.validate() | ||
| logger.info("Validating STAC document") |
There was a problem hiding this comment.
Should we allow validation to be toggled? Is a sensible default to always run validation, or only run it when desired?
There was a problem hiding this comment.
Yeah I think for production we would always want to validate. It failed for me on another scene but I haven't run enough test cases to understand the various issues we will run into - given we are just testing at the moment i thought it would be easier to keep off before doing a deeper investigation. Perhaps I should set a flag at the top though so Aman can do a bulk test on AWS later and we can work through the errors?
There was a problem hiding this comment.
I'm happy if you set the default behaviour to run the validation since it only logs the exception rather than exiting, right?
That sounds like a plan. 😊
There was a problem hiding this comment.
Nah the .validate() will exit the process as it raises an error that I then raise. I have added validate_stac as a flag at the top that we can set for the runs. We can later change to skip_validate_stac if we always want to validate without needing the flag.
There was a problem hiding this comment.
For testing then I feel like we should just do the stac-validation outside the container to avoid a whole bunch of jobs that processed the scene but had bad STAC. In production we should have stac-validation on like you said.
There was a problem hiding this comment.
Your commit 828ecfb is perfect. Please ignore my last comment :)
| self.stac_extensions = [ | ||
| "https://stac-extensions.github.io/product/v0.1.0/schema.json", | ||
| "https://stac-extensions.github.io/sar/v1.1.0/schema.json", | ||
| "https://stac-extensions.github.io/altimetry/v0.1.0/schema.json", |
There was a problem hiding this comment.
Is this extension just no longer needed? What were we using it for before?
There was a problem hiding this comment.
Yeah I dont think it made a lot of sense to use because it's not an altimeter. altm:instrument_mode-> sar:instrument_mode and the other value was removed
| # TODO fill with study result values | ||
| self.item.properties["nrb:geometric_accuracy_ALE"] = "TODO" | ||
| self.item.properties["nrb:geometric_accuracy_rmse"] = "TODO" | ||
| self.item.properties["nrb:geometric_accuracy_range"] = "TODO" | ||
| self.item.properties["nrb:geometric_accuracy_azimuth"] = "TODO" | ||
| self.item.properties["sarard:geometric_accuracy_ALE"] = "TODO" | ||
| self.item.properties["sarard:geometric_accuracy_rmse"] = "TODO" | ||
| self.item.properties["sarard:geometric_accuracy_range"] = "TODO" | ||
| self.item.properties["sarard:geometric_accuracy_azimuth"] = "TODO" |
There was a problem hiding this comment.
Can we remove these attributes from here and include them as typical values in our documentation for the product instead? It will prevent users from thinking they are scene-specific or burst-specific values.
There was a problem hiding this comment.
Good question, by definition I think they might reference the dataset as a whole, not just the scene/burst. In which case we should keep them. I'll need to check.
There was a problem hiding this comment.
I don't think we should keep the values if they reference the product as a whole.
If we do, we should say it's a typical constant value for ISCE3 from sample scenes.
sar_pipeline/aws/cli.py
Outdated
| burst_stac_manager.save(burst_folder / stac_filename) | ||
| # TODO validate the stac item when finalised | ||
| # burst_stac_manager.item.validate() | ||
| logger.info("Validating STAC document") |
There was a problem hiding this comment.
I'm happy if you set the default behaviour to run the validation since it only logs the exception rather than exiting, right?
That sounds like a plan. 😊
|
@caitlinadams @geoscience-aman I have added this flag. You can set on your runs, and not pass if we run into issues with validation but still want to make data for testing 828ecfb |
3 participants
