fiboa / specification

Field Boundaries for Agriculture (fiboa) - a specification that describes important properties of field boundaries
Apache License 2.0
9 stars 2 forks source link

Additional properties permitted or not? #17

Closed andyjenkinson closed 3 months ago

andyjenkinson commented 3 months ago

I notice that the spec description says that additional JSON properties other than those defined in the schema are not permitted. However the validator does not enforce that.

I think that the text is wrong and the validator is right based on the discussions in St Louis, but just in case you disagree let me make my case šŸ˜„

For additional properties to be considered invalid would be a dealbreaker I think. I understand why this would make sense for STAC, but I think is inappropriate for FIBOA. What it does is invalidate all existing standard GeoJSON data representations of boundaries and forces them to completely change their data model (rather than 'add' to it without breaking compatibility) and make extensions for every single property they want to expose. Unlike STAC that's a problem because these things already exist - we're building on top of an existing standard GeoJSON FeatureCollection, not inventing something new. So it is an unnecessary barrier to adoption. It's the same logic used for having as minimal core set of mandatory properties as possible, we don't want to invalidate lots of things. To put it clearly for us specifically: we would be forced to make "FIBOA specific" cut-down exports of the data, and could not adopt it into the core of the data provision and import process because there are too many other things we have that would break the data contract and which FIBOA will not be capable to represent for a long time.

Let's not make it harder to implement FIBOA than it needs to be. It really should be that the position is: "you can put what you like in here, but if you say this set of agreed things then it must follow these rules" Following Postel's law, any FIBOA consuming application is free to ignore any property it doesn't understand, but it's safe to apply consistent logic for those it does.

Incidentally you also lose the ability to see 'in the wild' what other properties are being commonly used, to inform future standardisation priorities, because you will never see a valid FIBOA specification that contains anything other than the existing defined core and extension properties.

So in summary: can we change the text so that additional properties are permitted? šŸ˜†

m-mohr commented 3 months ago

@andyjenkinson Which part of the spec are you referring to?

Is it the following in https://github.com/fiboa/specification/blob/main/core/validation.md ?

Note: In fiboa additional properties are disallowed (JSON Schema: additionalProperties: false).

Please note that this only refers to objects that we define, either in the core spec or extensions.

Let's say we define a new field called value_range, type object with a property min and max:

GeoJSON example:

{
  "type": Feature",
  "geometry": ...,
  "properties: {
    "area": 123,
    "value_range": {
      "min": 1,
      "max: 99
     }
  }
}

Schema:

value_range:
  type: object
  properties:
    min:
      type: number
    max:
      type: number

The quoted sentence above means that a user is not allowed to add e.g. a property called "mean" to the value_range object unless it's explicitly allowed in the schema for value_range. So if you'd want to allow that a user adds a mean property, define the schema as follows:

Schema:

value_range:
  type: object
  properties:
    min:
      type: number
    max:
      type: number
  additionalProperties: true

We definitively allow additional properties for fiboa itself (i.e. new GeoJSON properties / geoparquet columns). I hope this clarifies it.

m-mohr commented 3 months ago

I've made some changes to the document to make this a little more clear. I assume that this solves the issue and I'll close here. If that's not the case, leave a comment and I'm happy to reopen.

andyjenkinson commented 3 months ago

Ok fair enough, we just need to make sure never to define a schema for the root object itself (a GeoJSON Feature) or the properties object.

Can probably cope with it, it just means that any existing property already used by a dataset that happens to be an object (eg we have some) cannot be adopted into FIBOA without fully defining every key it contains. For example, in our view of a deduplicated boundaries, each "3rd party reference" to that boundary is represented as an array of objects, those objects may have metadata properties describing the boundary according to them and only some are under 'data model control' (ie we tell them if they give key X it should contain Y). The rest are freeform. This would have to sit out of scope of FIBOA because we can't know in advance what all of the properties are.

m-mohr commented 3 months ago

it just means that any existing property already used by a dataset that happens to be an object (eg we have some) cannot be adopted into FIBOA without fully defining every key it contains

That's not correct. Set additionalProperties to true and you can add whatever property you want without defining it.

andyjenkinson commented 3 months ago

Ok so you're only saying that the default assumption for the schema is for additionalProperties to be false, but can be changed - not that it is overridden to be true. Makes sense then, should be fine.

m-mohr commented 3 months ago

Exactly.