-
Notifications
You must be signed in to change notification settings - Fork 16
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
CityGML 3 Draft Review by 石丸伸裕 : Substantive Editorial Issues #119
Comments
Should we remove references to the Guide until we have a Guide document to point to? |
Re Nobu_E_8: Yes, the UML diagram "Core - Basic Types, Enumarations, Code lists, Class Diagram" from the EAP project should be added to 8.2.7. It's not only the Occupancy class that is missing, also the other classes should be displayed for better understanding of the UML model. |
The following sub-issues have been implemented in the UML model: The other sub-issues do not require updates to the UML model. |
Re E_4: I think we don't need to add a dependency to X3D. CityGML reuses concepts from X3D and modells them in the CityGML conceptual model, but the CityGML conceptual model does not depend on or refer to a X3D conceptual model. |
Re E_11 (first point): The role "storey" needs to be defined in BuildingUnit in the data dictionary. The figure is correct. |
@TatjanaKutzner The association between BuildingUnit and Storey is bi-directional with Storey being the source and BuildingUnit being the target. I believe this is the only bi-directional association in the model. I also thought we decided not to use bi-directional associations. If that is not the case, then I will have to update the EA templates. |
Recommended resolution: split the bi-directional BuildingUnit-Storey association into two unidirectional ones. Also need to compare the sub-issues in this issue with the current document and create a new comment with the unresolved items and possible solutions. |
The bi-directional association is now split up into two uni-directional associations the UML model. |
E_12: I added the INSPIRE specification on Buildings to Chapter 4. Referencing still needs to be done. |
Nobu_E_1: AbstractSpaceBoundary has been removed from Table 8 (Geometry). It does not appear in figure 18. However it does appear in figure 17 as well as table 7 (Space). |
Nobu_E_6, Nobu_E_9, Nobu_E_10: Additional ISO classes - See issue #118 |
E_1: I suggest to add AbstractPointCloud to figure 18 as it is also related to geometry representation. |
…s to upright format for better readability in the specification document
Implemented issue #119 + Changed layout of some diagrams to upright f…
E_1: AbstractPointCloud is now included in figure 18 |
Only open issue is inclusion of an example figure for Storey. That recommendation has been documented in issue #156. This issue can now be closed. |
In my opinion, these 12 issues require decisions or contributions of the original author(s) or the editors as a group.
Nobu_E_1: pp.49
Nobu_E_2: pp.51
Nobu_E_3: pp.55
Nobu_E_4: 8.3 Appearance
Nobu_E_5: 8.17
Nobu_E_6: 9.1
Nobu_E_7: pp.125 CityModel:
Nobu_E_8: pp.133 Occupancy:
Maybe the Occupancy should be added to Figure 19 or the UML diagram in eap file "Core - Basic Types, Enumarations, Code lists, Class Diagram" should be added to 8.2.7? [I would apologize if it is already shown in some UML diagrams in 8.2.]
Nobu_E_9: 9.6
Nobu_E_10: 9.15
Nobu_E_11: 9.17
ADEOfBuildingRoom: "None>" -> "None"
ADEOfBuildingUnit: The font of the Definition should be changed. [should be moved next row?]
pp.262 RoomHeight: "<←section,>>" -> [?]
Nobu_E_12: Annex D
The text was updated successfully, but these errors were encountered: