Actionable version #38
Conversation
… introduce distinction between local and external
jdreo
left a comment
jdreo
left a comment
There was a problem hiding this comment.
Awesome. Only a couple of comments on style.
.pre-commit-config.yaml
Outdated
| name: Check for patient ids starting with an "D". | ||
| always_run: true | ||
| args: [--multiline] | ||
| entry: 'D[0-9]{3}' |
There was a problem hiding this comment.
Why not a single [HMD][0-9]{3} ?
| row: | ||
| map: | ||
| columns: | ||
| - patient_id |
There was a problem hiding this comment.
Would be clearer with column: patient_id
| - tumorType | ||
| to_property: | ||
| - tumor_type | ||
| for_objects: |
There was a problem hiding this comment.
Same for for_object and to_property, when there is a single entry, it is not mandatory to use a list, you can just do to_property: tumorType.
weave.py
Outdated
| parser.add_argument("-i", "--clinical", metavar="CSV", nargs="+", | ||
| help="Extract from a clinical CSV file.") | ||
|
|
||
| parser.add_argument("-sml", "--short_mutations_local", metavar="CSV",nargs="+", |
There was a problem hiding this comment.
Convention is rather to use single-letter for short flags, and dash for long ones (instead of underscores).
There was a problem hiding this comment.
changed the underscores for the long flags. Concerning the short ones as single letter, since they won't be related to the long ones, I suggest not to remove them otherwise it will be confusing.
There was a problem hiding this comment.
Let's keep the acronyms for the short flags.
Co-authored-by: Marko Baric <marko.baric@pasteur.fr>
|
@jdreo
|
2 participants
First actionable version: