Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bands #5

Merged
merged 5 commits into from
Apr 26, 2024
Merged

Fix bands #5

merged 5 commits into from
Apr 26, 2024

Conversation

fmigneault
Copy link
Collaborator

@fmigneault fmigneault commented Apr 25, 2024

Description

Properly validate the bands reference under mlm:input to refer to corresponding eo / raster / STAC 1.1 "bands" definition.

Added

  • Add pattern for mlm:framework, needing at least one alphanumeric character,
    without leading or trailing non-alphanumeric characters.
  • Add examples/item_eo_and_raster_bands.json demonstrating the original
    use case represented by the previous examples/item_eo_bands.json contents.
  • Add a description field for mlm:input and mlm:output definitions.

Changed

  • Adjust scikit-learn and Hugging Face framework names to match the format employed by the official documentation.

Deprecated

  • n/a

Removed

  • Removed combination of mlm:input with bands: null that could never occur due to pre-requirement of type: array.

Fixed

  • Fix AnyBands definition and use in the JSON schema to better consider possible use cases with eo extension.
  • Fix examples/item_eo_bands.json that was incorrectly also using raster extension.
    This is not fundamentally wrong, but it did not allow to validate the eo extension use case properly, since
    the raster:bands reference caused a bypass for the mlm:input[*].bands to succeed validation.

Related Issue

n/a

Type of Change

  • 📚 Examples, docs, tutorials or dependencies update;
  • 🔧 Bug fix (non-breaking change which fixes an issue);
  • 🥂 Improvement (non-breaking change which improves an existing feature);
  • 🚀 New feature (non-breaking change which adds functionality);
  • 💥 Breaking change (fix or feature that would cause existing functionality to change);
  • 🔐 Security fix.

Checklist

  • I've read the CONTRIBUTING.md guide;
  • I've updated the code style using make codestyle;
  • I've written tests for all new methods and classes that I created;
  • I've written the docstring in Google format for all the methods and classes that I used.

@fmigneault fmigneault self-assigned this Apr 25, 2024
@fmigneault fmigneault marked this pull request as ready for review April 25, 2024 16:41
@fmigneault
Copy link
Collaborator Author

@rbavery FYI

Copy link
Collaborator

@rbavery rbavery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! I read over the band and statistics description in the README. When STAC 1.1 is released, I'm hoping we can make a stronger suggestion to use the MLM with this version since it simplifies the choice between how to describe bands. right now that section is pretty complex and I could see it tripping up folks who aren't super familiar with STAC.

@fmigneault
Copy link
Collaborator Author

@rbavery Ideally, people will migrate to STAC 1.1 over time, and then bands would be preferable. However, I don't want this to become a limiting factor in using MLM if for some reason a catalog is stuck on 1.0. For sure, we can increase the emphasis on STAC 1.1 when it's ready.

@fmigneault fmigneault merged commit 41f7ac3 into main Apr 26, 2024
9 checks passed
@fmigneault fmigneault deleted the fix-bands branch April 26, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants