cityjson / cjio

CityJSON/io: Python CLI to process and manipulate CityJSON files
MIT License
123 stars 32 forks source link

`info` command reports incorrect `cityobjects_total` #94

Closed balazsdukai closed 3 years ago

balazsdukai commented 3 years ago

cjio version 0.6.9 cityjson: Den Haag


command: cjio DenHaag_01.json subset --id "GUID_F784906B-FD5A-4F81-BACF-79C3566B2789_3" info

returns:

{
  "cityjson_version": "1.0",
  "epsg": 7415,
  "bbox": [
    78760.649,
    457966.139,
    4.105,
    78773.795,
    457977.664,
    8.035
  ],
  "transform/compressed": true,
  "cityobjects_total": 0,
  "cityobjects_present": [
    "BuildingPart"
  ],
  "materials": true,
  "textures": false
}

The problem is that cityobjects_total is 0, while there is one cityobject in the subset, as indicated by cityobjects_present, and I also verified it.


command: cjio tests/data/DenHaag_01.json subset --id 'GUID_3D7D60B9-8F3A-4D3B-A3E5-CD9B5565A5B2' --id 'GUID_F784906B-FD5A-4F81-BACF-79C3566B2789' --id 'GUID_F784906B-FD5A-4F81-BACF-79C3566B2789_1' --id 'GUID_F784906B-FD5A-4F81-BACF-79C3566B2789_2' --id 'GUID_F784906B-FD5A-4F81-BACF-79C3566B2789_3' info

returns: "cityobjects_total": 2 The problem is that all the 5 cityobjects are in the subset, they are just not counted.

I assume that the underscore-suffix on the identical ID-s is what messes up the counting.

hugoledoux commented 3 years ago

it's because BuildingPart is not considered a first_level object in the master branch. When we subset we leave the "parents": [ "GUID_F784906B-FD5A-4F81-BACF-79C3566B2789" ]. As it should I believe, although technically we break the file validity... @jliempt implement this (ec7721604495b290067ca0b17dcba846532b1034) and I agreed, but now I realise this. What to do?

The question is also: should info() report on first-level? I think not, with Extensions one could put all the Buildings into a Neighbourhood["children"] and then we have only but one cityobject_first level.

The branch develop has a "better" info(), it prints the total:

  "cityobjects_1stlevel": 1,
  "cityobjects_present": {
    "BuildingPart": 3,
    "Building": 1
  }

I think we should modify info() to print the hierarchy, eg:

|-- Neighbourhood (5)
   |-- Building (412)
      |-- BuildingPart (67)
   |-- TINRelief (1)
balazsdukai commented 3 years ago

The branch develop has a "better" info(), it prints the total:

Oh okay, I didn't realize that. I think that's fine.

I think we should modify info() to print the hierarchy,

That would be neat!

I'm closing the issue, because it is resolved on the develop branch already. And it wasn't even a bug.