-
Notifications
You must be signed in to change notification settings - Fork 7
Document public API #438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document public API #438
Conversation
|
@SKernchen Please review changes and approve as you see fit. |
SKernchen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the improvements. My suggestion is showing a more complex example. If we keep the short version, we should mention that "bar" is the expected name.
|
|
||
| #### Model instances in different types of plugin | ||
|
|
||
| You can extend `hermes` with plugins for three different commands: `harvest`, `curate`, `deposit`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So "process plugins" and "post-process plugins" won't exist any more? This should (also) be mentioned in some architectural doc rather than in a side note here. As far as I can tell it is not mentioned anywhere else yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made an issue for that #447 . In the parent issue it is also described.
|
|
||
| The complex data structure of JSON-LD is internally constructed in the `hermes` data | ||
| model, and to make it possible to work with only the data that is important - the actual terms | ||
| and their values - the internal data model types provide a function `.to_python()`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's all Python though 👀 Would .to_dict() be a better name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think .to_native() or .to_native_python() would be even better, since ld_dict is also a dict and this function also exists for ld_list.
But all suggestions do not mention that the structure is simplified in the process (similar, but not synonymous with compacting the expanded JSON-LD value).
|
Thanks for all of your comments. I'll address them after my holidays. |
8ace58c
into
refactor/423-implement-public-api
This PR adds explanatory documentation of the public API.
It complements #432