open-sdg / sdg-build

Python package to convert SDG-related data and metadata between formats
MIT License
5 stars 23 forks source link

SDMX output: StructureSpecific #218

Closed brockfanning closed 3 years ago

brockfanning commented 3 years ago

Fixes #205

With version 2.3.0 of the sdmx library, we can now produce StructureSpecific output. This PR makes this the default for Open SDG output, and also makes it an optional parameter.

brockfanning commented 3 years ago

@LucyGwilliamAdmin Not urgent, but I thought you might be interested in this. You can see this in action here: https://kazstat.github.io/sdg-data-kazstat/sdmx-output.html

LucyGwilliamAdmin commented 3 years ago

@brockfanning this is great! You can get a file with all data in as well??

brockfanning commented 3 years ago

@LucyGwilliamAdmin You mean the "all.xml"? Yep that is already a feature of the OutputSdmxMl class.

LucyGwilliamAdmin commented 3 years ago

@brockfanning I think you might have said somewhere else, but is structure specific required for SDG Lab?

brockfanning commented 3 years ago

@LucyGwilliamAdmin Yes it is.

LucyGwilliamAdmin commented 3 years ago

@brockfanning what's the best way to test this? I've merged these changes into my sdmx-mapping-2 branch and now using that on UK sandpit and structure specific xmls are being outputted.

I've just ran it through web SDMX converter to validate and all fine, so now about to try loading to SDG Lab to see if successful

LucyGwilliamAdmin commented 3 years ago

Ok so 5/5 individual files and the all file passed the validation. 5/5 individual files could be uploaded to SDG Lab but when I try to upload the all file to SDG Lab, it doesn't work but I don't know if it's supposed to?

Aside from that, is there anything else I should test?

LucyGwilliamAdmin commented 3 years ago

@brockfanning Daniel has just confirmed that SDG Lab accepts both single and multi-series files.

I think I see the problem and possibly not related to this - I've just revisited my multi-series file and it seems some parts of header are missing e.g. ID, Prepared, Sender:

https://onsdigital.github.io/sdg-sdmx-data/sdmx/all.xml

brockfanning commented 3 years ago

@LucyGwilliamAdmin Ah OK, makes sense. I think we can fix that in a separate PR.

LucyGwilliamAdmin commented 3 years ago

@brockfanning how do I configure whether or not structure specific is used?

I assumed I would need to add structure_specific option but I'm getting this error and I've tried setting it to False and false (both gave the same error): https://github.com/ONSdigital/sdg-sdmx-data/runs/2142432493?check_suite_focus=true#step:5:15

brockfanning commented 3 years ago

@LucyGwilliamAdmin For Open SDG I assumed that we would always want StructureSpecific, so I hardcoded that. I can remove the hardcoding if you think it should be optional.

LucyGwilliamAdmin commented 3 years ago

@brockfanning I think it's fine to have that as only option - I just seen in description that it had been added as an optional parameter so was just trying to test it

brockfanning commented 3 years ago

@LucyGwilliamAdmin Should I remove the concept_map stuff before merging?

Also I think I can tweak it so that structure_specific can be set to false without causing an error - with true still being the default.

LucyGwilliamAdmin commented 3 years ago

@brockfanning yes please remove the mapping stuff - forgot about that

I'm fine with it not being optional but if it's a quick thing, then I don't see the harm in it making it so - what are your thoughts?

brockfanning commented 3 years ago

@LucyGwilliamAdmin Making optional is easy - I've just pushed that, and also removed the concept_map code for now.