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

feat: validate undeclared SigMF extensions in metadata #104

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Arjunmehta312
Copy link

@Arjunmehta312 Arjunmehta312 commented Jan 20, 2025

Validate Extension Declarations in SigMF Files

Issue

Extensions used in SigMF metadata must be declared in the global core:extensions field, but the validator wasn't checking for undeclared extensions.

Changes

Added validation to ensure all extension namespaces used in the metadata are properly declared in global:core:extensions. The validator now:

  1. Extracts declared extension namespaces from global:core:extensions
  2. Scans through all metadata sections (global, captures, annotations) to find extension namespaces in use
  3. Raises a ValidationError if any extensions are used without being declared

Example

This will now fail validation:

{
"global": {
"core:datatype": "cf32_le",
"core:version": "1.0.0"
},
"captures": [{
"core:sample_start": 0,
"foo:some_field": 123 # Using undeclared 'foo' extension
}]
}

To fix, declare the extension:

{
"global": {
"core:datatype": "cf32_le",
"core:version": "1.0.0",
"core:extensions": [{
"name": "foo",
"version": "1.0.0",
"optional": true
}]
},
"captures": [{
"core:sample_start": 0,
"foo:some_field": 123 # Now properly declared
}]
}

Testing

  • Added tests to verify validation fails when extensions are used without declaration
  • Added tests to verify validation passes when extensions are properly declared
  • Tested with multiple extensions and nested fields

Related Issues

Implements feature request from monthly SigMF call to validate extension declarations as in issue and closes #103

@Teque5 Teque5 added the enhancement New feature or request label Jan 23, 2025
@Teque5
Copy link
Collaborator

Teque5 commented Jan 23, 2025

This is good but it will definitely cause some pipelines to break because we are being more strict about validation.

I believe we should merge since the spec says this field MUST exist, but I don't want to break other folk's existing code. Perhaps we should raise a warning for a while and then fail validation once we give people a few months?

I will have to update some of my own files where I omitted this field.

@gmabey
Copy link
Contributor

gmabey commented Jan 23, 2025

I suggest also waiting until after #324 has been merged (in addition to the warning period you described).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Basic Validation for Extensions
3 participants