Add documentation, fix some pylint errors

Author: pvanheus

README.md

@@ -4,9 +4,12 @@ Firstly, parser for the accession format described here: https://www.ncbi.nlm.ni
 `scrape_accession_rules.py`. This can be run as 
 
 ```
-scrape_accession_rules.py ncbi_accession_rules.json
+scrape_accession_rules.py accession_rules.json
 ```
 
 to scrape the rules at that website and save them in JSON format. The file generated by this parser is
 used in the second script, `parse_ncbi_accession.py` which can be used as a Python module but also
-as a command line tool e.g. `parse_ncbi_accession.py ncbi_accession_rules.json AE014297`.
+as a command line tool e.g. `parse_ncbi_accession.py accession_rules.json AE014297`.
+
+The `parse_ncbi_accession.py` script aims to work with all Python versions. The `scrape_accession_rules.py`
+script requires at least Python 3.6 and the modules specified in `conda_requirements.txt`.

ncbi_accession_rules.json accession_rules.json

@@ -0,0 +1,5 @@
+# This file may be used to create an environment using:
+# $ conda create --name <env> --file <this file>
+# platform: linux-64
+beautifulsoup4>=4.8.0
+requests>=2.22.0

conda_requirements.txt

@@ -1,29 +1,70 @@
 #!/usr/bin/env python
 
+"""Functions for parsing accessions and returning their source database and data type."""
+# Copyright (c) 2019, Peter van Heusden <[email protected]>
+# All rights reserved.
+
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in the
+#       documentation and/or other materials provided with the distribution.
+#     * Neither the name of the <organization> nor the
+#       names of its contributors may be used to endorse or promote products
+#       derived from this software without specific prior written permission.
+
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL Peter van Heusden BE LIABLE FOR ANY
+# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+from __future__ import print_function
 import json
 import re
 
 class RuleMatcher(object):
-    def matches(this, accession):
+    """Superclass for matcher objects that have a match() method to return whether they match an accession."""
+
+    def matches(self, accession):
+        """matches(self, accession)
+
+        Returns true of accession matches this rule."""
         pass
 
 class RangeMatcher(RuleMatcher):
+    """RuleMatcher subclass that can match an accession whose prefix is in a range of letter prefixes."""
 
     def __init__(self, prefix, suffix_length):
-        self.re = re.compile(r'{prefix}[A-Z]{suffix_length}')
-    
-    def matches(this, accession):
+        # this is not a greedy match so we need to specify that it is followed by a number
+        self.re = re.compile(f'{prefix}[A-Z]{suffix_length}' + r'\d')
+
+
+    def matches(self, accession):
         return self.re.match(accession) is not None
 
 def make_range_matcher(spec):
+    """make_range_matcher(spec)
+
+    spec - string (e.g. 'BAAA-BZZZ')
+
+    Turns a range specification into a object that can match an accession that falls
+    within that range. Returns a RangeMatcher() object"""
     if '-' in spec:
         (start, end) = spec.split('-')
     elif '_' in spec:
         (start, end) = spec.split('_')
     else:
         raise ValueError('require specification with - or _ in it, got: ' + spec)
     assert end.endswith('Z'), f'Range specifier has unknown format: {spec}'
-    for i in range(len(start)):
+    for i, _ in enumerate(start):
         if start[i] != end[i]:
             break
     prefix_length = i
@@ -33,6 +74,14 @@ def make_range_matcher(spec):
 
 
 def build_accession_parser(rules_file):
+    """build_accession_parser(rules_file)
+
+    rules_file - file object open for reading
+
+    Builds a rule parse usable by match_accession() in this module from the accession rules
+    described in rules_file. These rules can be downloaded by the scrape_accession_rules
+    script."""
+
     rules_data = json.load(rules_file)
     rules_by_prefix_len = {}
     for prefix_list, database, type_description in rules_data:
@@ -49,11 +98,17 @@ def build_accession_parser(rules_file):
                 rules_by_prefix_len[prefix_length].append((prefix, database, type_description))
     return rules_by_prefix_len
 
-letter_re = re.compile('[A-Z]+')
+# letter_re is a greedy match so we do not need to specify that the letter prefix is followed by a number
+LETTER_RE = re.compile('[A-Z]+')
 def match_accession(accession, rules_by_prefix_len):
-    if accession[2] == '_':
-        return ('NCBI', 'RefSeq')
-    letter_match = letter_re.match(accession)
+    """match_accession(accession, rules_by_prefix_len)
+    
+    Returns the tuple (database, accession_type, type_description) for accession using the rules specified by
+    rules_by_prefix_len. The database is one of 'GenBank', 'NCBI', 'GenBank and DDBJ' , 'DDBJ', and 'EMBL',
+    accession_type is one of 'nucleotide', 'protein', 'WGS' and 'MGA' and the type_description is the description
+    of the type of data associated with that accession."""
+
+    letter_match = LETTER_RE.match(accession)
     if letter_match is None:
         raise ValueError('an accession number must start with at least one capital letter, this does not: ' + accession)
     letter_prefix = letter_match.group(0)
@@ -66,18 +121,21 @@ def match_accession(accession, rules_by_prefix_len):
         accession_type = 'nucleotide'
     elif letter_match_length < 4:
         accession_type = 'protein'
-    elif letter_match_length == 4 or letter_match_length == 6:
+    elif letter_match_length in (4, 6):
         accession_type = 'WGS'
     elif letter_match_length == 5:
         accession_type = 'MGA'
     else:
         raise ValueError('an accession number must start with less than 7 capital letters, this does not: ' + accession_type)
-    
+
+    if accession[2] == '_':
+        return ('NCBI', accession_type, 'RefSeq')
+
     rules = rules_by_prefix_len[letter_match_length]
     for rule in rules:
         (matcher, database, type_description) = rule
         if (isinstance(matcher, RuleMatcher) and matcher.matches(accession)) or letter_prefix == matcher:
-            return (database, type_description)
+            return (database, accession_type, type_description)
 
 if __name__ == "__main__":
     import argparse
@@ -86,4 +144,4 @@ def match_accession(accession, rules_by_prefix_len):
     parser.add_argument('accession')
     args = parser.parse_args()
     rules_by_prefix_len = build_accession_parser(args.rules_data_file)
-    print(match_accession(args.accession, rules_by_prefix_len))
+    print(match_accession(args.accession, rules_by_prefix_len))

parse_ncbi_accession.py

@@ -1,4 +1,37 @@
 #!/usr/bin/env python
+"""Script for parsing accession description page and saving to JSON.
+
+Usage: scrape_accession_rules.py <output file>
+
+e.g.
+
+scrape_accession_rules.py accession_rules.json
+"""
+
+# Copyright (c) 2019, Peter van Heusden <[email protected]>
+# All rights reserved.
+
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in the
+#       documentation and/or other materials provided with the distribution.
+#     * Neither the name of the <organization> nor the
+#       names of its contributors may be used to endorse or promote products
+#       derived from this software without specific prior written permission.
+
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL Peter van Heusden BE LIABLE FOR ANY
+# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import argparse
 import json
@@ -8,19 +41,16 @@
 import requests
 
 
-# def iterate_id_range(spec: str) -> List[str]:
-#     (start, end) = spec.split('-')
-#     assert end.endswith('Z'), f'Range specifier has unknown format: {spec}'
-#     for i in range(len(start)):
-#         if start[i] != end[i]:
-#             break
-#     prefix_length = i
-#     assert prefix_length < len(start), f'Prefix not found in range specifier {spec}'
-#     suffix_length = len(start) - prefix_length
+def parse_rules(text: str) -> List[List[List[str], str, str]]:
+    """parse_rules(text: str) -> List[List[List[str], str, str]]:
+       
+       parse the rules from the NCBI webpage, returns a list of lists, which each inner list having the
+       elements:
 
-def parse_rules(text: str) -> List[List[str]]:
-    """parse_rules(text: str):
-       parse the rules from the NCBI webpage"""
+       prefix - a list of accession prefixes that this rule applies to
+       database - name of the database or databases associated with this rule
+       type_description - description of the type of data associated with this rule
+       """
     soup = BeautifulSoup(text, 'html.parser')
     data = []
     for table in soup.find_all('table', cellpadding='3'):
@@ -36,15 +66,21 @@ def parse_rules(text: str) -> List[List[str]]:
     return data
 
 
-def fetch(url='https://www.ncbi.nlm.nih.gov/Sequin/acc.html') -> str:
+def fetch(url: str = 'https://www.ncbi.nlm.nih.gov/Sequin/acc.html') -> str:
+    """fetch(url:str) -> str
+
+    Fetches the accession type description page (by default from https://www.ncbi.nlm.nih.gov/Sequin/acc.html)"""
     response = requests.get(url)
-    if response.status_code == requests.codes['ok']:
-        return response.text
-    else:
+    if response.status_code != requests.codes['ok']:
         raise requests.exceptions.RequestException(f'Failed to download {url}', response=response)
+    return response.text
+
 
+def save_data(data: List[List[List[str], str, str]], output: TextIO) -> None:
+    """save_data(data: List[List[List[str], str, str]], output: TextIO)
 
-def save_data(data: List[List[str]], output: TextIO) -> None:
+    Saves the data from parsing the accession description page to a JSON format file. output must be a
+    file open for writing."""
     json.dump(data, output, indent=2)
     output.close()