cityjson / specs

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

Misleading ADE documentation #29

Closed clausnagel closed 5 years ago

clausnagel commented 5 years ago

I just read through the Extensions chapter of the CityJSON 0.8 specification on http://www.cityjson.org and came across the following:

While Extensions are less flexible than CityGML ADEs (ie they have a narrower scope and less customisation is possible), it should be noted that the flexibility of ADEs come at a price: standard software (eg viewer or spatial analysis software) will not process correctly the files containing ADEs since specific code needs to be written.

I would not agree with the "specific code needs to be written" part of this statement. It really depends on how the parser is implemented. It is, of course, possible to implement a generic GML parser that can handle arbitrary GML schemas. Since both CityGML and ADEs are just GML schemas, such a parser would be able to read ADE-enriched CityGML datasets without having to write any specific code.

There is software using generic GML parsers such as FZKViewer or FME, or libraries such as GeoTools or GDAL. Both FZKViewer and FME, for instance, can read and visualize ADE datasets without additional code.

There is also software for which the statement is true such as the citygml4j library. But this is not because of a general limitation of ADEs, but rather because citygml4j uses an XML binding approach instead of a generic GML parser. And, of course, it's the same in the JSON world: if you use a JSON binding approach to derive classes from the core CityJSON schemas, then support for any CityJSON Extension would require specific extra code to be written.

liberostelios commented 5 years ago

I think this is correct.

In theory, it is be possible to parse any GML schemas as soon as you have a general GML parsing approach. In practice, though, many peculiar cases need specific code anyway. At least that's my experience using GDAL. It already had issues dealing with complicated mechanisms of the CityGML data model, which had to be identified after thorough testing against some dataset. Similar issues also happened with IMGEO data. I am pretty sure there are still CityGML files out there that cannot be fully parsed by GDAL and I would safely assume that the same applies to ADEs as well.

In any case, I think the text should be altered as the statement is indeed misleading. Instead, I would point to the fact that extensions tend to favour composition over inheritance, which is considered a good practice in OOP. Also, I think the word "flexible" might be misused here, as I think JSON extensions are more flexible but ADEs are more powerful.

kenohori commented 5 years ago

I see your point here @clausnagel and agree that the text is a bit of an oversimplification. However, I think that the target here is not us (who are happy to read the full spec and implement a complete parser), but a casual user who just wants to implement something quick and dirty using the native JSON support in JS/Python/Ruby. For them, it's nice to be able to get support for extensions essentially for free (e.g. by iterating over all CityObjects and doing something with their geometry regardless of their type). I think that's what the text is aiming at.

clausnagel commented 5 years ago

Ok, but still it's not correct that ADEs cannot be processed without specific extra code.

hugoledoux commented 5 years ago

I changed the text: c67b489a6a8a274fc50f7a956b25ca15f20525c5

But in general having ADEs means that if a software says it supports CityGML, then it might not support the specific files. This is a misunderstanding among practitioners that we meet: they spend time/money creating an ADE, and then they realise that most software/libraries (except viewers, since it's simple to just look for geometries in a file) are incapable to process their files... We want CityJSON to be for users and practitioners, not for developers, and thus the doc should be for them IMO.

clausnagel commented 5 years ago

Thanks @hugoledoux, much appreciated.