phac-nml/staramr

Change strict restriction on PointFinder organisms to be a warning

apetkau opened this issue · 2 comments

apetkau commented

Currently, running with --pointfinder-organism ORGANISM with anything other than salmonella or campylobacter will result in an error due to a check I perform against a list of acceptable organisms here:

staramr/staramr/blast/pointfinder/PointfinderBlastDatabase.py

Lines 78 to 83 in 107e2cd

def get_available_organisms(cls):
"""
A Class Method to get a list of organisms that are currently supported by staramr.
:return: The list of organisms currently supported by staramr.
"""
return ['salmonella', 'campylobacter']

This was done to ensure that I only allow results to be generated from PointFinder databases/organisms that I have validated work with staramr. But, there are additional mutations from the PointFinder databases belonging to other organisms that could still be useful to run with staramar, even if not every mutation (or indel) will work properly.

Instead of restricting which organisms can be used by staramr for PointFinder I wish to switch this to a warning. That is, if --pointfinder-organism ORGANISM is set to anything outside of the list of validated organisms, results will still be generated by staramr, but a warning message will be printed.

pimarin commented

Hi @apetkau,
Thanks to yours comments, I take time to answer to learn python and oriented object programming to understand your work !
My first conclusion was the same as you.

  • Instead of using the get_available_organisms(cls) method maybe the get_organisms(cls, database_dir) already existing do the job ! It use the information from the config file into pointfinder directory and you already created a test to check diff between organism
    I'm just trying to understand how to refer to the new method
  • Then for pointfinder nucleotide analysis, I will try to translate all nucleotide position into positive value (as you mentioned for promoter region with negative position) to be use in your code. I think some changes in the CodonMutationPosition.py to be more flexible ?
apetkau commented

Hello @pimarin,

That's awesome you've taken the time to learn Python and Object Oriented programming 😄. I hope it is going well for you. I can be a very useful language to learn. Another important library I use throughout staramr is pandas for manipulation of data in Python (in particular, loading and working with CSV files or other tabular data structures). And then, while I don't use these in staramr, this could lead into learning about matplotlib or seaborn for creating visual plots of data. But this is all quite a bit to learn (and is a bit outside of what staramr handles).

  1. Yes, the get_organisms() method is used to load up all the PointFinder organism name available in the currently-installed version of the PointFinder database:

    staramr/staramr/blast/pointfinder/PointfinderBlastDatabase.py

    Lines 86 to 94 in fd4c230

    def get_organisms(cls, database_dir):
    """
    A Class Method to get the list of organisms from the PointFinder database root directory.
    :param database_dir: The PointFinder database root directory.
    :return: A list of organisms.
    """
    config = pd.read_csv(path.join(database_dir, 'config'), sep='\t', comment='#', header=None,
    names=['db_prefix', 'name', 'description'])
    return config['db_prefix'].tolist()

    Just to clarify, the reason I use get_available_organisms() instead of get_organisms() to return a list of organisms to use for PointFinder is because anything not returned by get_available_organisms() is not guaranteed to work in staramr. That is it was a decision I made to prevent staramr from returning potentially invalid results. This particular issue would be a compromise where instead of disallowing people from running staramr with organisms not returned by get_available_organisms(), I would simply print a warning.

  2. Thanks so much for offering to help out with looking into supporting those other coordinates for PointFinder databases (negative coordinates or insertions/deletions). I really do appreciate it. I would welcome any contributions you can make.

    However, note that I have someone in my group who will be looking into this issue too. I want to be a bit more open about my plans for staramr, so I have started to write up my plans in the Projects section of GitHub. There are two projects you can see here:

    1. Release 0.8.0: This will update the default databases for staramr and I will try to include fixing this issue (where you can run staramr with other PointFinder organisms, but you will get a warning printed).
    2. Release 1.0.0: This will include full support for other PointFinder organisms by supporting insertions/deletions/promotor regions leading to AMR.

    I hope this all makes sense? Again, I would welcome any suggestions or contributions you may have. But we do plan to work on fixing up some of these issues and packaging them up into new staramr releases.