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

Add MLM extension #1542

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Add MLM extension #1542

wants to merge 25 commits into from

Conversation

jonas-hurst
Copy link
Contributor

@jonas-hurst jonas-hurst commented Apr 2, 2025

Related Issue(s):

Description:
Adds the STAC:MLM extension: https://github.com/stac-extensions/mlm

PR Checklist:

  • Pre-commit hooks pass (run pre-commit run --all-files)
  • Tests pass (run pytest)
  • Documentation has been updated to reflect changes, if applicable
  • This PR maintains or improves overall codebase code coverage.
  • Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

@jonas-hurst jonas-hurst marked this pull request as draft April 2, 2025 13:23
Copy link

codecov bot commented Apr 2, 2025

Codecov Report

Attention: Patch coverage is 99.48119% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.34%. Comparing base (e713adf) to head (10b7c93).

Files with missing lines Patch % Lines
pystac/extensions/mlm.py 99.47% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1542      +/-   ##
==========================================
+ Coverage   91.62%   92.34%   +0.72%     
==========================================
  Files          54       55       +1     
  Lines        7578     8349     +771     
  Branches      923      959      +36     
==========================================
+ Hits         6943     7710     +767     
- Misses        451      453       +2     
- Partials      184      186       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jonas-hurst jonas-hurst marked this pull request as ready for review April 3, 2025 13:15
@jonas-hurst jonas-hurst marked this pull request as draft April 3, 2025 13:17
@jonas-hurst jonas-hurst marked this pull request as ready for review April 3, 2025 13:37
@gadomski gadomski self-requested a review April 7, 2025 13:17
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Some minor tweaks requested, mostly spelling.

Comment on lines +9 to +11

### Fixed
- fixed missing parameter "title" in pystac.extensions.classification.Classification ([#1539](https://github.com/stac-utils/pystac/pull/1539))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Fixed
- fixed missing parameter "title" in pystac.extensions.classification.Classification ([#1539](https://github.com/stac-utils/pystac/pull/1539))

Comment on lines +231 to +234
@property
def mlm(self) -> MLMExtension[Item]:
return MLMExtension.ext(self.stac_object)

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this needs to be added to CollectionExt too.

Comment on lines +115 to +127
REGRESSION = ("regression",)
CLASSIFICATION = ("classification",)
SCENE_CLASSIFICATION = ("scene-classification",)
DETECTION = ("detection",)
OBJECT_DETECTION = ("object-detection",)
SEGMENTATION = ("segmentation",)
SEMANTIC_SEGMENTATION = ("semantic-segmentation",)
INSTANCE_SEGMENTATION = ("instance-segmentation",)
PANOPTIC_SEGMENTATION = ("panoptic-segmentation",)
SIMILARITy_SEARCH = ("similarity-search",)
GENERATIVE = ("generative",)
IAMGE_CAPTIONING = ("image-captioning",)
SUPER_RESOLUTION = "super-resolution"
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason that some of these are one-tuples and others are strings?

name: Name of the band referring to an extended band definition
format: The type of expression that is specified in the expression property
expression: An expression compliant with the format specified.
The cxpression can be applied to any data type and depends on the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The cxpression can be applied to any data type and depends on the
The expression can be applied to any data type and depends on the

Comment on lines +470 to +474
type: The type of ValueScaling operation for which required proreties are
to be retrieved

Returns:
list[str]: names of proreties required for the given ``type``
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type: The type of ValueScaling operation for which required proreties are
to be retrieved
Returns:
list[str]: names of proreties required for the given ``type``
type: The type of ValueScaling operation for which required properties are
to be retrieved
Returns:
list[str]: names of properties required for the given ``type``

cls, type: ValueScalingType, props: dict[str, Any]
) -> None:
"""
Validate whether given properties satisfy the requiremts set by the ValueScaling
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Validate whether given properties satisfy the requiremts set by the ValueScaling
Validate whether given properties satisfy the requirements set by the ValueScaling

MLMExtension[T]: The extended object

Raises:
TypeError: When a :class:`pystac.Asset` object is apssed as the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
TypeError: When a :class:`pystac.Asset` object is apssed as the
TypeError: When a :class:`pystac.Asset` object is passed as the

@property
def accelerator_constrained(self) -> bool | None:
"""
Get or set the accelerator_constrianed property
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Get or set the accelerator_constrianed property
Get or set the accelerator_constrained property

"""
Get or set this asset's compile_method property
"""
return self.properties.get(COMPILE_MDTHOD_ASSET_PROP)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return self.properties.get(COMPILE_MDTHOD_ASSET_PROP)
return self.properties.get(COMPILE_METHOD_ASSET_PROP)

"""
Get or set this asset's entrypoint property asdfasdf
"""
return self.properties.get(ENTRYPOITN_ASSET_PROP)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return self.properties.get(ENTRYPOITN_ASSET_PROP)
return self.properties.get(ENTRYPOINT_ASSET_PROP)

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.

Add support for stac:mlm extension
2 participants