search

cityjson / specs

Specifications for CityJSON, a JSON-based encoding for 3D city models
https://cityjson.org
Creative Commons Zero v1.0 Universal
108 stars 25 forks source link

Some proposed fixes for v09 beta schemas #37

Closed clausnagel closed 6 years ago

clausnagel commented 6 years ago

While going through the v09 beta schemas, I found some issues.

  1. BuildingInstallation has the same attributes as Building. Since this was not the case in v06 and v08 (and is also not the case in the CityGML data model), I removed these attributes.

  2. According to the v08 spec, TransportSquare should have the same geometries as Road and Railway (MultiSurface and CompositeSurface). But it additionally has GeometryInstance, so I removed this one.

  3. Since Road, Railway, and TransportSquare have the same attributes and geometries (after step 2), I added a new abstract type _AbstractTransportationComplex (which is inline with the CityGML model as well).

  4. Abstract types like _AbstractBuilding or _AbstractBridge do not inherit the attributes from _AbstractCityObject. In constrast, the concrete type Building then inherits from both _AbstractCityObject and _AbstractBuilding.

    Fixed this so that _AbstractBuilding now inherits from _AbstractCityObject, and Building from _AbstractBuilding. I think this is important because if someone wants to create a new building type in an extension and if they want to extend from _AbstractBuilding for this purpose, they now inherit the common attributes from _AbstractCityObject as well.

hugoledoux commented 6 years ago

Step 1-2-3 are great, thanks for fixing my error (copy-and-paste...) and for improving the Transportation

clausnagel commented 6 years ago

Well, I did the changes for step 4 with oXygen, and using its validate functionality did not give me any error. But then I thought that oXygen maybe only performs a JSON syntax check and not a full schema validation (I am using quite an outdated version 16.1...).

So, I just merged the separate CityJSON schemas into one file cityjson-v09beta.json. Afterwards, I copied this into the https://www.jsonschemavalidator.net/ online validator. It does not give errors, and I can successfullly validate CityJSON datasets. It looks good, but please give it another try with your tools.

hugoledoux commented 6 years ago

indeed, it does not give errors.

but: add an error (add "class": 22,) for instance to the attributes of a Building, then that error will not be detected (unless I got lost in all my tests..)

clausnagel commented 6 years ago

I used this simple CityJSON dataset test.json and put "class": 22 into it as suggested. https://www.jsonschemavalidator.net/ reports an error. Well, in fact it reports 60 errors... But here my JSON knowledge ends, so you can better judge whether this makes sense or not.

image

hugoledoux commented 6 years ago

Mea culpa. It seems that through my tests I concluded wrongly that this doesn't work, but it must have been caused by another error or whatever. Because now indeed it works, I checkout out your branch and tested with your files and others. All good 👍🏽

I don't know about your 60 errors to be honest; I use cjio to validate, which calls python-json-schema. If you use the $ cjio test.json validate --folder_schemas /home/elvis/myschemas/v09/ it is easy to see what works and what not.

So I'll accept the PR, thanks for it!