search

open-contracting / ocdskit

A suite of command-line tools for working with OCDS data
https://ocdskit.readthedocs.io
BSD 3-Clause "New" or "Revised" License
17 stars 7 forks source link

add package-records command #109

Closed duncandewhurst closed 4 years ago

duncandewhurst commented 4 years ago

closes #98

I needed this to load some data from Afghanistan into the Data Review Tool.

I think I've covered everything in the contributing guidelines, the only bit I couldn't get working was the tests for test_command_root_path_array and test_command_root_path_item so I've commented those out.

Edit: in retrospect, given there is a lot of duplication of the package-releases command, we might just want to either have a generic package command or just update the documentation to explain the package-releases can also be used to package records.

Edit 2: I am also not sure of the best way to add an argument to populate the record package's packages key, since the command already accepts a list of extensions.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.07%) to 95.431% when pulling bc8a525cb22eb80618c25ecd1fc8de316a029b31 on package-records into cc64b884a0fc2d67c230d60ed3d29967b6d11e5b on master.

jpmckinney commented 4 years ago

Is there a use case for setting packages? In what case would a user have unpackaged records, but know the release packages from which they are sourced?

jpmckinney commented 4 years ago

I eliminated the duplication by extracting a private _package method in the library.

We could have a flexible package command, but I think it's better and clearer to be specific.

If all looks good, feel free to merge.

duncandewhurst commented 4 years ago

Thanks @jpmckinney

No immediate need for setting packages, it was just for completeness.