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

Attribute extensions #31

Closed
clausnagel opened this issue Oct 12, 2018 · 13 comments
Closed

Attribute extensions #31

clausnagel opened this issue Oct 12, 2018 · 13 comments

Comments

@clausnagel
Copy link
Contributor

clausnagel commented Oct 12, 2018

The CityJSON extension specification discusses two possible alternatives for adding new attributes to a City Object: 1) simply add the new attributes to the "attributes" property, or 2) define a new City Object (recommended).

Things get tricky if you have two or more extensions that want to add new attributes to, for instance, a "Building", and if you want to share them in one CityJSON file. Option 1) will fail if attributes from one extension happen to share the same name with predefined CityJSON attributes or attributes from another extension. You would have to adapt their names just for being able to exchange them. Option 2) will not fail but you would have to exchange, for instance, a "+NoiseBuilding" and an "+EnergyBuilding" with all the same "geometry" and "children" and so on. I think this is a huge overhead if you just want to add some noise and energy attributes.

So I would like to propose another solution. Why not allow adding a new property for each extension, which then contains all additional attributes from this extension for a predefined City Object? To be consistent, the names of these new properties should also start with a "+". For example:

{
  "type": "Building",
  "attributes": { ... },
  "+noise-attributes": {
    "level": 1,
    ...
  },
  "+energy-attributes": {
    "level": "AAA",
    ...
  },
  "geometry": [...]
}

The benefit is that predefined CityJSON attributes in "attributes" and attributes from the different extensions are nicely seperated, which avoids name clashes. And you do not have to define multiple City Objects just for attributes. Furthermore, the additional properties like "+noise-attributes" or "+energy-attributes" could be defined in their own JSON schema.

You could also wrap those attribute containers by another predefined property:

{
  "type": "Building",
  "attributes": { ... },
  "extension-attributes": [
    "+noise-attributes": {
      "level": 1,
      ...
    },
    "+energy-attributes": {
      "level": "AAA",
      ...
    }
  ],
  "geometry": [...]
}

This adds an extra level of nesting to the data structure and thus is more verbose. But the positive aspect is that "extension-attributes" is defined in the CityJSON schema. It would thus be the only extension point for adding new attributes to a predefined City Object. So this might be helpful for implementers, as they would only have to check "extension-attributes" for additional attributes.

@liberostelios
Copy link
Contributor

I think this is a very nice idea. Having a predefined single point of extension attributes, like the proposed "extension-attributes", is probably the best way to standardise the process.

As far as I understand, though, it can only be offered as a "best practice" suggestion, as JSON schemas cannot force/validate all those rules. For instance, the extension creator can only standardise the attributes of the "+noise-attributes" object, but cannot specify which types of city object can have this attribute. In addition, JSON schemas cannot really validate whether extension attributes are inside the "extension-attributes" or not (I think).

So, in the end, fully complying with this will be up to the developer/user. But I still think this is the best possible solution and maybe it should be encouraged through the documentation.

@hugoledoux
Copy link
Member

I totally agree with the downside you mention @clausnagel. However, as Stelios mentioned, unfortunately I don't think it's possible to do with the JSON Schemas, at least not with "vanilla ones". The concept of inheritance does not exist as in XML. So one would have to overwrite "Building" type, but this is not what we want.

The only way would be to define ourselves the mechanism, that is we "stitch" the attributes in a given file to the "Building" type, it's rather easy to do but I hesitate to go this way. The validation becomes less generic, and one can't validate with a generic JSON Schema validator.

However, it should be said that with the Extensions CityJSON has already gone down that path... the validation is a 2-step process: (1) generic CityObjects; (2) the Extensions are validated one-by-one. So if we all like the idea, it's possible to do so.

Do we?

@clausnagel
Copy link
Contributor Author

Ok, I understand that a standard JSON Schema validator would not work. So, the cjio validation is a specific implementation to be able to deal with CityJSON extensions?

As @liberostelios wrote, I would also be happy if the approach would only be encouraged through the CityJSON documentation in a first step.

Just because I'm curious: If you take the CityJSON schemas plus a set of extension schemas that follow a well-defined structure, then it should be possible to write a small software that combines them and outputs a valid JSON Schema file. This could then be fed to a standard JSON Schema validator in order to validate a given dataset. The extension "schemas" would not even have to be fully compliant JSON schemas themselves, but could just be snippets. Is this correct? If so, sounds doable.

@hugoledoux
Copy link
Member

That's a brilliant idea.

I indeed already stitch and resolve the references to build one schema (from the different files), so if there was a small script stitching the different parts (defined in JSON, in a structured way) into one schema that should work. And this would work with generic JSON Schema validators.

@clausnagel
Copy link
Contributor Author

clausnagel commented Oct 12, 2018

Cool. To complete this thought, there could be a nice validation process:

  1. A CityJSON file must provide references to all extension "schemas" (structured, JSON-based) used in the file
    (very much like the current "extensions" property)
  2. Using these references, a parser can create an integrated JSON schema
    (by using own code, a lib/software or - what I like most - by calling a web service)
  3. Using this schema, a parser can validate the file
    (again by using own code, a default JSON validation lib/software or some online tool)

Sounds pretty flexible and strong.

@kenohori
Copy link
Member

I love the idea of having a script that merges all schemas to validate everything at once.

Now, getting back to the original topic, I think having a single point of entry to get all extension attributes is much better than having to iterate through all the children of a City Object.

That being said, in most cases I'd honestly prefer to just have extension attributes inside "attributes". A simple best practice of prepending them with the extension name could solve clashes (i.e. "noise-level", "energy-level"). This fits better with the CityJSON philosophy IMHO and would ensure that software supports many extensions automatically.

@clausnagel
Copy link
Contributor Author

clausnagel commented Oct 17, 2018

Well, the current CityJSON 0.8 extension specification explicitly discourages putting extension attributes inside "attributes" but recommends to document them in their own JSON schema by defining a new City Object. In the Noise ADE mapping example, a "+NoiseBuilding" is created for this purpose, but the extension attributes still go into a new "noise-attributes" container instead of "attributes". I think this is overly complex if you just want to add some attributes to a "Building", and I do not see how this would enable CityJSON software to automatically handle extension attributes.

I do agree though that putting all extension attributes inside "attributes" is a valid alternative to my proposal. And I also acknowledge that having long attribute names is a JSON way to at least reduce the probability of name clashes. So, I also like this proposal. I think the CityJSON specification should then state that any (simple or complex) extension attribute should go inside "attributes", no matter whether you deal with predefined or newly defined City Objects. This would enable CityJSON software to handle extension attributes automatically. It still might be beneficial to require the name of an extension attribute to start with a "+".

I think independent of whether using "attributes" or "extension-attributes" it should be possible to define extension attributes in a way that would allow creating a single JSON schema for validation.

@hugoledoux
Copy link
Member

hugoledoux commented Nov 20, 2018

ok, I took the "favourite way" from above and implemented it, it's in the branch 'development'.

The schemas have been all revamped, ie now they use _AbstractObjects which are reused for the Extensions; it's now simpler and less error-prone to extend objects for Extensions.

I too prefer to have all the attributes inside "attributes" only (single point of entry for developers), and this is the behaviour I forced for adding new attributes to existing City Objects. Of course if one defines a new CityObject she can have another list, but if it's only extending the attributes of a CityJSON Objects then it's must inside "attributes".

All new attributes/properties/CityObjects start with +; the only question left is whether we force the "namespace" before the attributes as Claus discussed above. I would leave it to the person making the Extension to decide. What you think?

You can read the draft of Extensions online: https://www.cityjson.org/en/development/extensions

@hugoledoux
Copy link
Member

Oh and I forgot: stitching the schemas together is doable, but takes more time than I had thought... so not available yet, I'll do this at some point.

cjio validate is being updated, so the branch 'develop' works (beta version...) for the v09 schemas and extensions.

@clausnagel
Copy link
Contributor Author

clausnagel commented Nov 21, 2018

all, great work, thanks.

@clausnagel
Copy link
Contributor Author

clausnagel commented Nov 21, 2018

One question while reading the draft specification: In the first attribute example, color does not start with a +. Is this by intention? Is a + only required if the attribute is defined in an extension schema?

{
  "type": "Building",
  "attributes": {
    "storeysAboveGround": 2,
    "colour": "red"
  },
  "geometry": [...]
}

@hugoledoux
Copy link
Member

I want to allow new attributes because JSON makes it easy. This attribute can be whatever you want. Not an error, but you'll get a warning (implementation to follow soon).

Only if you want to document in a schema an attribute, then do you have to put the "+".

@clausnagel
Copy link
Contributor Author

Thanks, understood. And +1 from me.

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

No branches or pull requests

4 participants