Open-Telecoms-Data / lib-cove-ofds

Other
0 stars 0 forks source link

Check - all nodes must be used in a link? #5

Open odscjames opened 2 years ago

odscjames commented 2 years ago

From https://github.com/Open-Telecoms-Data/lib-cove-ofds/issues/1#issuecomment-1270116319

All nodes must be used in a link?

From https://github.com/Open-Telecoms-Data/lib-cove-ofds/issues/1#issuecomment-1270278003

I don't think we do want to say this. There may be edge cases where nodes are unattached to a link for good reason, and from a graph theory perspective floating nodes don't present a problem

odscjames commented 2 years ago

The only use case I can think of so far is that Phase 1 includes building some nodes, and Phase 2 which isn't fully mapped or described yet is going to connect them.

[ I could also see one network operator running 2 totally independent networks that aren't actually connected (maybe on different sides of a country or something) but this check wouldn't cover that anyway.]

How about we do this but style it as a warning instead of an error?

It's possible that any occurrences of this are more likely to be a result of bad data mapping / entry than a real world edge case, and thus the warning would be helpful?

lgs85 commented 2 years ago

Yes, a warning would be helpful. @duncandewhurst suggested the following:

It's not a normative rule but it could be a useful 'sense check' with a message like:

Your data contains nodes that are not referenced by any links. You should check that this is not the result of an error in your mapping or data pipeline.

duncandewhurst commented 2 years ago

Agree it should be a warning not an error. In the supply-side research we saw examples of datasets that were effectively lists of nodes without information about links.

duncandewhurst commented 2 years ago

Looks good on the testing server. Just need to update the table to have the correct columns per the additional checks spreadsheet from https://github.com/Open-Telecoms-Data/cove-ofds/issues/13#issuecomment-1302762080

duncandewhurst commented 2 years ago

Testing on 2022-11-08 with this file, CoVE fails to report the unreferenced node.

odscjames commented 1 year ago

I can't actually find this in spread sheet any more, but I think this is now on live and ready to test?

Test file: https://raw.githubusercontent.com/Open-Telecoms-Data/lib-cove-ofds/main/tests/fixtures/pythonvalidate/node_not_used_in_any_spans_1.input.json

duncandewhurst commented 1 year ago

It's the first check in the spreadsheet, but at some point we renamed it from 'node references' to 'unreferenced nodes' - I've updated the spreadsheet now.

I'm still not seeing a failure reported when testing with the file shared in my previous comment: https://ofds.cove.opendataservices.coop/data/7ea82278-5f8e-439a-9407-0103e75e7331

Could that be because there are two nodes with the same .id in that file?

Edit: It's working OK with the test file that you shared, though.