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

Proper way to handle existing 'optional' field when required for an extension #123

Open
jacobagilbert opened this issue May 25, 2021 · 6 comments
Assignees
Labels
clarification a wording change in the spec that clarifies meaning enhancement

Comments

@jacobagilbert
Copy link
Member

jacobagilbert commented May 25, 2021

SigMF has [intentionally] created many 'optional' fields within the core namespace and throughout the extensions. Its highly likely that certain applications may require some of these optional fields, for which there are three reasonable paths I see:

  • Not Handled by Spec: this is the current behavior where the necessary but optional-per-the-spec fields 'should be there' and their absence is handled via input sanitization where assumptions can be made or the offending portions dropped.
  • Extension Redefinition: extensions can redefine optional core namespace objects as required (strictly, the opposite should not be allowed).
  • Extension Overloading: extensions should redefine required objects identical to those in the optional core (or extension) as members of a new extension namespace.

I lean toward one of the last one, as it provides a spec-controlled method for managing this situation and does not allow extensions to modify objects outside their scope, though having said that the proposed multichannel extension does modify core field behavior. Regardless of what the 'correct' way to handle this is, I feel like the spec should provide guidance on the best practice here.

@bhilburn
Copy link
Contributor

Agreed, @jacobagilbert. FWIW, so far our policy has been that canonical extensions are able to add new fields to core, but no other extensions are permitted to do so.

NOTE TO SELF: the above fact should also be stated in the spec 😅

Mulling over it. I don't super care for extensions duplicating existing fields in new namespaces (your last option) -- what if an app puts different data in the core and extension versions of that field, which is correct? Right now, I'm leaning towards the second option. Open to hearing other opinions, too.

@djanderson - as someone that's done a lot of work in an important extension, do you have thoughts?

@bhilburn bhilburn added this to the Release v1.0.0 milestone May 26, 2021
@bhilburn bhilburn added clarification a wording change in the spec that clarifies meaning enhancement labels May 26, 2021
@bhilburn bhilburn self-assigned this May 26, 2021
@bhilburn bhilburn changed the title Proper way to handle existing 'optional' field when required for an application Proper way to handle existing 'optional' field when required for an extension May 26, 2021
@jacobagilbert
Copy link
Member Author

jacobagilbert commented May 26, 2021

Ok, given that

our policy has been that canonical extensions are able to add new fields to core

I would lean toward the second option also, however this will also need language permitting non-canonical extensions to make optional core fields required (no other modifications should be necessary) as the intent here is to enable end applications to unambiguously define what is required (which are inherently unlikely to be canonical extensions).

A specific example is a user application that requires spectral boundaries on annotations. There are sane ways to address this in the application as per #1 but it would be better IMO to provide a mechanism to unambiguously describe the required data via an extension.

@bhilburn
Copy link
Contributor

Thinking about my previous statement more. We actually don't have any extensions that have added anything to core (despite our previous policy), and actually, I think we should maybe reconsider that stance. Put differently, I think the policy should actually be "core namespace fields may only be defined by the primary spec".

Regardless of that, though, thinking about this further, I'm wondering if this is something that makes sense to address in an extension. From a user's perspective, for example, if I want to use fields in an extension with a recording without the core field's boolean changed, does that mean I can't use any other field in that extension? It's not really the extension that requires it so much as the application itself. I'm not saying I like the option in the first bullet, but conceptually I think I'm more aligned there.

On the flip-side, there, though, if a specific application requires an extension to work, it would be good to have some mechanism through which to indicate that -- including overriding the core booleans.

Hrm. Not sure where to go with this one right now. Will spin a bit. Ideas?

@jacobagilbert
Copy link
Member Author

We can let this spin. I currently employ method 1 + an app note, which is not horrible or anything, but it does feel like something we could possibly address in the spec.

FWIW, I also do not think extensions should modify core values, and that is one concern i have for the proposed multichannel extension.

@gmabey
Copy link
Contributor

gmabey commented Jun 18, 2021

@jacobagilbert what part of this issue still pertains to Release v1.0.0?

@jacobagilbert
Copy link
Member Author

It does not; for v1.0 this will be handled outside the spec.

I don't have permission to edit the Project @bhilburn created but I updated the Milestone to reflect v2.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification a wording change in the spec that clarifies meaning enhancement
Projects
None yet
Development

No branches or pull requests

3 participants