Make the plugin ready for the new data model#3
Conversation
|
Hey @notactuallyfinn, here is another case where @SKernchen and I refactored a HERMES plugin to use the new data model. Would you be willing to take a look at the code to see if this makes sense? Thanks! 😄 |
notactuallyfinn
left a comment
There was a problem hiding this comment.
Looks good to me, only a few very minor things.
|
|
||
| def prepare(self): | ||
| def __call__( | ||
| self, command: HermesCommand, metadata: SoftwareMetadata |
There was a problem hiding this comment.
The annotation of command could be changed to HermesCurateCommand from hermes.commands.curate.base, which is more precise.
|
|
||
| def create_report(self): | ||
| """Create basic text report.""" | ||
| def create_report(self, metadata: SoftwareMetadata): |
There was a problem hiding this comment.
metadata is never used in the method.
| ctx = HermesCacheManager() | ||
| validation_file = ctx.cache_dir / "curate" / "validation.json" | ||
| validation_file.parent.mkdir(exist_ok=True, parents=True) | ||
| self._validation_graph.serialize(validation_file, format="json-ld") |
There was a problem hiding this comment.
Why not use the HermesCacheManager the way it was intended to be used?
(validation_file would have to be moved, but is there another reason?)
| validation_file.parent.mkdir(exist_ok=True, parents=True) | ||
| self._validation_graph.serialize(validation_file, format="json-ld") | ||
|
|
||
| self._report = create_report(self._validation_graph) |
There was a problem hiding this comment.
self._report seems to be unused.
Can it be removed?
Known bug: The new data model uses
schema: <http://schema.org/>while rdflib used in the validation code usesschema: <https://schema.org/>which causes validation to be skipped for all non-https things.