Skip to content

Commit

Permalink
geojsontojson: Include name in organisation and phase references
Browse files Browse the repository at this point in the history
#33

And
* check id is a string
* reorder and comment code to make it easier to read
  • Loading branch information
odscjames committed Nov 22, 2022
1 parent 4221901 commit 615fe10
Show file tree
Hide file tree
Showing 12 changed files with 167 additions and 71 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- add more information to errors
- Add unique ID checks
- Add path to output
- GeoJSON to JSON:
- Include name in organisation and phase references

## [0.4.0] - 2022-11-09

Expand Down
107 changes: 64 additions & 43 deletions libcoveofds/geojson.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import copy
from collections import defaultdict
from typing import Optional

from json_merge_patch import create_patch as json_diff_function

Expand Down Expand Up @@ -231,11 +232,11 @@ def _process_network(self, geojson_feature_node_or_span: dict) -> None:
):
out: list = []
for phase_reference in contract["relatedPhases"]:
phase_id = self._process_phase(
phase_data = self._process_phase(
network.get("id"), phase_reference
)
if phase_id:
out.append({"id": phase_id})
if phase_data:
out.append(phase_data)
else:
out.append(phase_reference)
contract["relatedPhases"] = out
Expand All @@ -255,15 +256,15 @@ def _process_node(self, geojson_feature_node: dict) -> None:
# sort organisations
for field in ["physicalInfrastructureProvider", "networkProvider"]:
if isinstance(node.get(field), dict) and node.get(field):
organisation_id = self._process_organisation(network_id, node[field])
if organisation_id:
node[field] = {"id": organisation_id}
organisation_data = self._process_organisation(network_id, node[field])
if organisation_data:
node[field] = organisation_data

# sort phase
if isinstance(node.get("phase"), dict) and node.get("phase"):
phase_id = self._process_phase(network_id, node["phase"])
if phase_id:
node["phase"] = {"id": phase_id}
phase_data = self._process_phase(network_id, node["phase"])
if phase_data:
node["phase"] = phase_data

if geojson_feature_node.get("geometry"):
node["location"] = geojson_feature_node["geometry"]
Expand All @@ -285,15 +286,15 @@ def _process_span(self, geojson_feature_span: dict) -> None:
# sort organisations
for field in ["physicalInfrastructureProvider", "networkProvider", "supplier"]:
if isinstance(span.get(field), dict) and span.get(field):
organisation_id = self._process_organisation(network_id, span[field])
if organisation_id:
span[field] = {"id": organisation_id}
organisation_data = self._process_organisation(network_id, span[field])
if organisation_data:
span[field] = organisation_data

# sort phase
if isinstance(span.get("phase"), dict) and span.get("phase"):
phase_id = self._process_phase(network_id, span["phase"])
if phase_id:
span["phase"] = {"id": phase_id}
phase_data = self._process_phase(network_id, span["phase"])
if phase_data:
span["phase"] = phase_data

if geojson_feature_span.get("geometry"):
span["route"] = geojson_feature_span["geometry"]
Expand All @@ -303,38 +304,58 @@ def _process_span(self, geojson_feature_span: dict) -> None:

self._networks[network_id]["spans"].append(span)

def _process_phase(self, network_id: str, phase: dict) -> str:
def _process_phase(self, network_id: str, phase: dict) -> Optional[dict]:
phase_id = phase.get("id")
if phase_id:
if phase_id in self._networks[network_id]["phases"]:
# Is it inconsistent with what we have seen before?
if json_diff_function(
self._networks[network_id]["phases"][phase_id], phase
):
self._inconsistent_phase_ids_by_network_id[network_id].add(phase_id)
else:
self._networks[network_id]["phases"][phase_id] = phase
return phase_id
return ""
# If no id, can't do anything. TODO log somewhere?
if not phase_id or not isinstance(phase_id, str):
return None
# Check data
if phase_id in self._networks[network_id]["phases"]:
# Is it inconsistent with what we have seen before?
if json_diff_function(
self._networks[network_id]["phases"][phase_id], phase
):
self._inconsistent_phase_ids_by_network_id[network_id].add(phase_id)
else:
# Not seen this before; store it
self._networks[network_id]["phases"][phase_id] = phase
# Make output
out: dict = {"id": phase_id}
# Take name from data on network, not data that is passed to this function.
# This means that if inconsistent names are in input, we'll have consistent names in the output.
name = self._networks[network_id]["phases"][phase_id].get("name")
if name:
out["name"] = name
return out

def _process_organisation(self, network_id: str, organisation: dict) -> str:
def _process_organisation(
self, network_id: str, organisation: dict
) -> Optional[dict]:
organisation_id = organisation.get("id")
if organisation_id:
if organisation_id in self._networks[network_id]["organisations"]:
# Is it inconsistent with what we have seen before?
if json_diff_function(
self._networks[network_id]["organisations"][organisation_id],
organisation,
):
self._inconsistent_organisation_ids_by_network_id[network_id].add(
organisation_id
)
else:
self._networks[network_id]["organisations"][
# If no id, can't do anything. TODO log somewhere?
if not organisation_id or not isinstance(organisation_id, str):
return None
# Check data
if organisation_id in self._networks[network_id]["organisations"]:
# Is it inconsistent with what we have seen before?
if json_diff_function(
self._networks[network_id]["organisations"][organisation_id],
organisation,
):
self._inconsistent_organisation_ids_by_network_id[network_id].add(
organisation_id
] = organisation
return organisation_id
return ""
)
else:
# Not seen this before; store it
self._networks[network_id]["organisations"][organisation_id] = organisation
# Make output
out: dict = {"id": organisation_id}
# Take name from data on network, not data that is passed to this function.
# This means that if inconsistent names are in input, we'll have consistent names in the output.
name = self._networks[network_id]["organisations"][organisation_id].get("name")
if name:
out["name"] = name
return out

def get_json(self) -> dict:
out: dict = {"networks": []}
Expand Down
24 changes: 16 additions & 8 deletions tests/fixtures/geojson_to_json/organisations_1.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
"id": "1",
"name": "Accra",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111"
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
},
"networkProvider": {
"id": "GH-RGD-CS222222222"
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
},
"location": {
"type": "Point",
Expand All @@ -26,10 +28,12 @@
"name": "Kumasi",
"status": "operational",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111"
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
},
"networkProvider": {
"id": "GH-RGD-CS222222222"
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
},
"location": {
"type": "Point",
Expand All @@ -47,10 +51,12 @@
"start": "1",
"end": "2",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111"
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
},
"networkProvider": {
"id": "GH-RGD-CS222222222"
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
},
"route": {
"type": "LineString",
Expand Down Expand Up @@ -114,11 +120,13 @@
"organisations": [
{
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
"name": "FibreCo",
"country": "IE"
},
{
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
"name": "FastWeb",
"country": "IE"
}
]
}
Expand Down
15 changes: 15 additions & 0 deletions tests/fixtures/geojson_to_json/organisations_1.meta.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,18 @@
"/networks/nodes/physicalInfrastructureProvider/id": {
"count": 2
},
"/networks/nodes/physicalInfrastructureProvider/name": {
"count": 2
},
"/networks/nodes/networkProvider": {
"count": 2
},
"/networks/nodes/networkProvider/id": {
"count": 2
},
"/networks/nodes/networkProvider/name": {
"count": 2
},
"/networks/nodes/location": {
"count": 2
},
Expand Down Expand Up @@ -63,12 +69,18 @@
"/networks/spans/physicalInfrastructureProvider/id": {
"count": 1
},
"/networks/spans/physicalInfrastructureProvider/name": {
"count": 1
},
"/networks/spans/networkProvider": {
"count": 1
},
"/networks/spans/networkProvider/id": {
"count": 1
},
"/networks/spans/networkProvider/name": {
"count": 1
},
"/networks/spans/route": {
"count": 1
},
Expand All @@ -86,6 +98,9 @@
},
"/networks/organisations/name": {
"count": 2
},
"/networks/organisations/country": {
"count": 2
}
},
"inconsistent_phases_by_network_id": {},
Expand Down
12 changes: 8 additions & 4 deletions tests/fixtures/geojson_to_json/organisations_1.nodes.geo.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
"name": "Accra",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
"name": "FibreCo",
"country": "IE"
},
"networkProvider": {
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
"name": "FastWeb",
"country": "IE"
},
"network": {
"id": "a096d627-72e1-4f9b-b129-951b1737bff4",
Expand All @@ -42,11 +44,13 @@
"status": "operational",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
"name": "FibreCo",
"country": "IE"
},
"networkProvider": {
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
"name": "FastWeb",
"country": "IE"
},
"network": {
"id": "a096d627-72e1-4f9b-b129-951b1737bff4",
Expand Down
6 changes: 4 additions & 2 deletions tests/fixtures/geojson_to_json/organisations_1.spans.geo.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,13 @@
},
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
"name": "FibreCo",
"country": "IE"
},
"networkProvider": {
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
"name": "FastWeb",
"country": "IE"
},
"network": {
"id": "a096d627-72e1-4f9b-b129-951b1737bff4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
"id": "1",
"name": "Accra",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111"
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
},
"networkProvider": {
"id": "GH-RGD-CS222222222"
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
},
"location": {
"type": "Point",
Expand All @@ -26,10 +28,12 @@
"name": "Kumasi",
"status": "operational",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111"
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
},
"networkProvider": {
"id": "GH-RGD-CS222222222"
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
},
"location": {
"type": "Point",
Expand All @@ -47,10 +51,12 @@
"start": "1",
"end": "2",
"physicalInfrastructureProvider": {
"id": "GH-RGD-CS111111111"
"id": "GH-RGD-CS111111111",
"name": "FibreCo"
},
"networkProvider": {
"id": "GH-RGD-CS222222222"
"id": "GH-RGD-CS222222222",
"name": "FastWeb"
},
"route": {
"type": "LineString",
Expand Down
Loading

0 comments on commit 615fe10

Please sign in to comment.