Fixes #106 #117
Fixes #106 #117
Conversation
Adding support for instrument-based configuration. Since the configuration file is defined in mg-scripts, but not used by mg-scripts, there is not much change required here.
Pull Request Test Coverage Report for Build 7562474734
💛 - Coveralls |
| # SN – RapidRun which is HiSeq 2500 | ||
| 'SN': 'RapidRun'} | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
why staticmethod vs. a regular method? It would allow to use self vs. InstrumentUtils.types[code] ...
There was a problem hiding this comment.
This was a hedge. I know that I naturally tend to resort to creating classes more than others in the lab do. In this case I did want to encapsulate the types within a scope, but I also demonstrated that none of the methods require state to be maintained in between them.
I wouldn't recommend making these regular methods for that reason. If anything I would recommend making types and the methods global instead of making them regular methods.
IMHO my first choice would be to migrate these to metapool, since they would be potentially useful in notebooks and other code. I can go either way on either keeping it a static class or making them global.
There was a problem hiding this comment.
True, and I'm not sure why classes you prefer them and if you are the only one why not follow: "Vox Populi, Vox Dei".
Anyway, I think we will also need these values for other things/projects; for example, this is important information to calculate the resource allocations for these jobs - basically, calculate the actual values that will go to the config file. Following this idea and @wasade comments, maybe adding a single tsv file with the values of the dict would be a better solution so we can expand in the future, when needed. What do you think?
Co-authored-by: Daniel McDonald <[email protected]>
Co-authored-by: Daniel McDonald <[email protected]>
Co-authored-by: Daniel McDonald <[email protected]>
|
@wasade I'm hesitant to change how we determine the type and ID since everyone seems to remember this method and it does appear durable. However I can start pulling examples of xml files from across the runs to see if we can pull it reliably from there if you guys think it's worth it. We don't really have a tool to parse these and that may be a good thing to have yes? |
|
Some googling suggests the instrument lookup may not be universally correct, and presumably Illumina or IGM has authoritative information. For regex, doesn't this work? >>> import re
>>> matcher = re.compile(r'(\d{6,8})_([A-Z0-9]+)_(\d+)_([A-Z0-9]+)')
>>> matcher.search('231201_A01535_0431_BHVKWCDSX7').groups()
('231201', 'A01535', '0431', 'BHVKWCDSX7') |
|
Superceded by #123 |
Addresses #106