feat: ✨ add write_package_properties()
#984
Conversation
Co-authored-by: Luke W. Johnston <lwjohnst86@users.noreply.github.com>
Co-authored-by: Signe Kirk Brødbæk <40836345+signekb@users.noreply.github.com>
Co-authored-by: Signe Kirk Brødbæk <40836345+signekb@users.noreply.github.com>
…checks-2-simple-helper-functions
…t/sprout-checks-7-check-properties
Co-authored-by: Luke W. Johnston <lwjohnst86@users.noreply.github.com>
…eck-error-matcher
| def write_package_properties(path: Path, properties: PackageProperties) -> Path: | ||
| """Writes the specified package properties to the `datapackage.json` file. | ||
|
|
||
| Writes a set of package properties (with or without resource properties) back to the |
There was a problem hiding this comment.
As we're rewriting the entire file/object, it made sense for me to allow resource properties to be present but to not require them necessarily. Maybe this (and all our recent changes) will allow us to simplify write_resource_properties.
There was a problem hiding this comment.
Yea, I'm currently going through the design interface data package section. Does it make sense that this function overwrites the file? It keeps it simple. But how might we keep it from overwriting the resources if there are any? In those situations where they read -> edit -> write.
There was a problem hiding this comment.
I think that with our current setup, you are not expected to use the write_package_properties function to edit AND write, as you could, as you say, accidentally delete your resource properties.
But if you edit THEN write, then it's all good, because edit_package_properties doesn't modify existing resource properties.
More generally, maybe it would make more sense to have a single function for writing to the datapackage.json file. Then it would be very obvious that whatever you give to that function will be written to file as is, no merging, no clever updates. This function pretty much does this already.
I don't think we're likely to implement a write function that touches only the bits of the file we want to change?
There was a problem hiding this comment.
So I understand what you're saying. You mean we have a single write_properties() function, that overwrites everything? Yea, that makes sense. Seems that it might be a good idea to include some version controlling here though...!!! That's for later hahah mmmmmm.
And then we have the edit_ function to make edits to specific fields that we give to write_properties()?
There was a problem hiding this comment.
Yeah!! I put this random comment on another PR that pretty much says the same thing 😁
| def test_rewrites_resource_properties_when_file_exists(path, package_properties): | ||
| """Should write properties to file when the file already exists, updating existing | ||
| resource properties.""" | ||
| old_properties = replace( |
There was a problem hiding this comment.
Just found this function, pretty useful!! 🛺
| """Should throw `CheckError`s if there are errors in the package properties.""" | ||
| package_properties.name = "invalid name with spaces" | ||
| package_properties.id = None | ||
| write_json(resource_properties.compact_dict, path) |
There was a problem hiding this comment.
Just to test that the file is not rewritten when the check fails.
| check_properties( | ||
| properties, | ||
| ignore=[CheckErrorMatcher(validator="required", json_path="resources")], | ||
| ) |
| def write_package_properties(path: Path, properties: PackageProperties) -> Path: | ||
| """Writes the specified package properties to the `datapackage.json` file. | ||
|
|
||
| Writes a set of package properties (with or without resource properties) back to the |
There was a problem hiding this comment.
Yea, I'm currently going through the design interface data package section. Does it make sense that this function overwrites the file? It keeps it simple. But how might we keep it from overwriting the resources if there are any? In those situations where they read -> edit -> write.
Co-authored-by: Luke W. Johnston <lwjohnst86@users.noreply.github.com>
lwjohnst86
changed the title
write_package_propertieswrite_package_properties()
Description
This PR adds the
write_package_propertiesfunction. This will be used to complete the package guide.This PR needs an in-depth review.
Checklist
just run-all