fix a typo in metadata field #749
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #749 +/- ##
===========================================
- Coverage 46.16% 46.08% -0.08%
===========================================
Files 78 78
Lines 7047 7059 +12
===========================================
Hits 3253 3253
- Misses 3794 3806 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Release notes for the metadata change
While you're doing that, could you update release notes for all the changes since the last release?
There was a problem hiding this comment.
In the main release note? Yea. I am planning to do it with the BBMap PR.
There was a problem hiding this comment.
ok, as long as the changes from this pr are included
| metadata = {'tool_name': 'checkm2', | ||
| 'version': '1.0.1', | ||
| 'command': command, | ||
| "reference_db": { | ||
| "version": None, | ||
| "comment": "diamond_db, ver unknown", | ||
| }, | ||
| 'ids_to_files': ids_to_files} | ||
| 'ids_to_files': make_json_serializable(ids_to_files), | ||
| 'run_time': round(run_time, 2), | ||
| 'batch_size': size, | ||
| } |
There was a problem hiding this comment.
Wouldn't it make more sense, now I think about it, to handle this outside the container? Then the code would live in one place rather than having to duplicate it for every container
There was a problem hiding this comment.
hmmm which block of code are you referring to? Generate the metadata outside of container? something like making create_tool_metadata takes command, run_time, size, etc.?
There was a problem hiding this comment.
Generate the metadata outside of container?
right. The running code should know all this stuff, so you could abstract it all out of the containers
There was a problem hiding this comment.
This is something that could probably be done later though, as I assume it'd be a big lift
There was a problem hiding this comment.
👍 I moved everything to create_tool_metadata.
There was a problem hiding this comment.
Sorry, missed this.
I mean the metadata code doesn't run in the container at all, and is written by code after the container exits. Then you don't need to add it to every tool, it Just Works
There was a problem hiding this comment.
That probably won’t be a good idea. If we move this logic completely outside of container, we will need if statement for each tool for tool specific metadata. And For ‘command’ metadata, we also need to return the command.
There was a problem hiding this comment.
I don't think so, most of the additional metadata is either source file, data id, ref db version, or ids_to_files, which is either known or can be looked up by looking in the tool directory. The only anomaly is mash, and for that there could be an method in each tool dir, that if it exists, takes the input args and returns the extra metadata.
For the command, the command = fn(input_args) so there could also be a method that does that translation that is used both in the container to start the executable and outside the container to generate the metadata.
Anyway, that's a different PR if it gets done at all.
No description provided.