Open-Telecoms-Data / lib-cove-ofds

Other
0 stars 0 forks source link

GeoJSON -> Json: Include name in phase and org references #33

Closed odscjames closed 1 year ago

odscjames commented 2 years ago

Wasn't sure about this and kept meaning to ask as either is technically valid and there isn't an equivalent of https://open-fibre-data-standard.readthedocs.io/en/0.1-dev/reference/publication_formats/geojson.html?highlight=geojson#geojson-transformation-specification to follow (should there be?)

But https://github.com/Open-Telecoms-Data/cove-ofds/issues/5#issuecomment-1309384150 says:

.name is missing from all the phase references. .name is missing from all the organisation references

duncandewhurst commented 2 years ago

Yes, let's include .name.

As a principle, I think that we should aim for GeoJSON/JSON to be round-trippable, like the CSV format, i.e. after converting a file from JSON to GeoJSON, if you convert it back again, you should get the same file you started with.

I've opened an issue on documenting a specification for the GeoJSON to JSON transformation: https://github.com/Open-Telecoms-Data/open-fibre-data-standard/issues/185

odscjames commented 2 years ago

Yes, let's include .name.

ok

As a principle, I think that we should aim for GeoJSON/JSON to be round-trippable

Pedantically noting that will be impossible for json->geojson->json. Start with JSON without names in refs, process it, it will now have names in refs. (I think?)

I suspect that actually a better principle is to make the best data we can. If there was an error in the original data, should we try and preserve it when roundtripping? I'm not sure why that would be desirable

Maybe this can be a :canned_food: of :worm: :worm: :worm: discussion for later - I'll bear it in mind tho if anything else comes up where this might be related.

duncandewhurst commented 2 years ago

Good points :-)

Pedantically noting that will be impossible for json->geojson->json. Start with JSON without names in refs, process it, it will now have names in refs. (I think?)

Yes, that sounds right.

I suspect that actually a better principle is to make the best data we can. If there was an error in the original data, should we try and preserve it when roundtripping? I'm not sure why that would be desirable

Given that CoVE basically only validates the JSON version of the data (as I understand it), I don't think that we want to explicitly fix errors in either of the conversions to JSON . Doing so would effectively disguise the errors whereas we want the publisher to be aware of them and fix them. I'm less concerned if the conversions from JSON do things like fill in missing names in refs.

Aside from missing names in refs, I haven't thought through what other scenarios this might apply to, but I can think about that more when I come to document a specification for the GeoJSON to JSON conversion.

odscjames commented 2 years ago

I don't think that we want to explicitly fix errors in either of the conversions to JSON . Doing so would effectively disguise the errors

Ah, I meant fix AND have a warning. In the same way the GeoJSON->JSON already gives you a warning for inconsistent references in the meta output eg https://github.com/Open-Telecoms-Data/lib-cove-ofds/blob/v0.4.0/tests/fixtures/geojson_to_json/organisations_inconsistent_1.meta.expected.json#L92

Then in Cove, we can decide how to surface those warnings to the user. I agree we don't want to disguise them.

Aside from missing names in refs, I haven't thought through what other scenarios this might apply to

I haven't either, but it's kinda related to https://github.com/Open-Telecoms-Data/cove-ofds/issues/36

duncandewhurst commented 2 years ago

Sounds good!

odscjames commented 1 year ago

Name inclusion now ready for testing on live!

duncandewhurst commented 1 year ago

Looks good!