Repository: acorg/seqgen

Issue:
Simplify sequencing naming so everything has a name (id)

Discussion:

Patch:
diff --git a/README.md b/README.md
index 40f7376..9ca6f87 100644
--- a/README.md
+++ b/README.md
@@ -70,26 +70,24 @@ As an example, the JSON shown below specifies the following:
     },
     "sequences": [
         {
-            "name": "A",
             "id": "seq-A",
             "length": "%(length)d"
         },
         {
-            "from name": "A",
+            "from id": "seq-A",
             "id prefix": "seq-A-mutant-",
             "length": "%(length)d",
             "count": 19,
             "mutation rate": 0.01
         },
         {
-            "name": "B",
             "id": "seq-B",
-            "from name": "A",
+            "from id": "A",
             "length": "%(length)d",
             "mutation rate": 0.15
         },
         {
-            "from name": "B",
+            "from id": "seq-B",
             "id prefix": "seq-B-mutant-",
             "length": "%(length)d",
             "count": 19,
@@ -99,12 +97,12 @@ As an example, the JSON shown below specifies the following:
             "id": "recombinant",
             "sections": [
                 {
-                    "from name": "A",
+                    "from id": "seq-A",
                     "start": 1,
                     "length": 30
                 },
                 {
-                    "from name": "B",
+                    "from id": "seq-B",
                     "start": 31,
                     "length": 70
                 }
@@ -129,29 +127,30 @@ There are many more small examples of usage in
 The sequence specification must be a list of objects.  The full list of
 specification keys you can put in a object in the JSON is as follows:
 
-* `count`: The number of sequences to generate from this object.  If the
-  sequence specification also contains a `name`, the name will refer to the
-  first of the sequences generated by this object.
+* `alphabet`: A string of characters from which sequences will be drawn
+  (see `random nt` and `random aa` below).
+* `count`: The number of sequences to generate from this object.
 * `description`: The sequence description. This will be appended to the
-  FASTA id (separated by a space).
+  FASTA id (separated by a space). Note that if a sequence has a
+  description and you want to refer to it using `from id` (see below), you
+  must include its id and description, separated by a single space.
 * `id`: The FASTA id to give this sequence.
 * `id prefix`: The prefix of the FASTA ids to give a set of sequences. A
   count will be appended to this prefix. This is useful when you specify a
   `count` value.
-* `from name`: The sequence should come from another (already named)
-  sequence in the JSON file. You give a name to a sequence using the `name`
-  key (below).
+* `from id`: The sequence should be based on another (already named)
+  sequence in the JSON file. The value given should either be the exact
+  `id` of another sequence or else be the `id prefix` of another sequence
+  followed by a number. E.g., if you specify `"count": 10` and `"id
+  prefix": "my-id"` for a set of ten sequences, and want to refer to the
+  3rd of them when specifying another sequence, you would use `"from id":
+  "my-id-3"`. The default id prefixes for sequences that are not given an
+  id explicity is `seq-id-`.
 * `length`: The sequence length.
 * `mutation rate`: A mutation rate to apply to the sequence.
-* `name`: Give a name to the sequence so you can refer to it from elsewhere
-  in the JSON. Note that this has nothing to do with FASTA ids, it is just
-  an internal name.
-* `name count`: An optional integer specifying which of a set of sequences
-  generated from the same specification should be named (assumes `name`
-  is also given). The default is to name the first sequence. Note that this
-  number is 1-based. Special values `first` and `last` are also allowed.
 * `random aa`: The sequence should be made of random amino acids.
-* `random nt`: The sequence should be made of random nucleotides.
+* `random nt`: The sequence should be made of random nucleotides. This is
+  the default.
 * `ratchet`: If `true` and a count and mutation rate are given, sequences
   after the first will be mutants of the previous sequence. This can be
   used to build a series of sequences that are successive mutants of one
@@ -167,7 +166,7 @@ specification keys you can put in a object in the JSON is as follows:
   only the first sequence in the file is used.
 * `skip`: If `true` the sequence will not be output. This is useful either
   for temporarily omitting a sequence or for just giving a sequence (e.g.,
-  one read from a file) a name so part of it can be used as a section of
+  one read from a file) an id so it can be used in the construction of
   another sequence.
 
 All specification keys are optional. A completely empty specification
diff --git a/seqgen/__init__.py b/seqgen/__init__.py
index 51d6e92..3614796 100644
--- a/seqgen/__init__.py
+++ b/seqgen/__init__.py
@@ -2,6 +2,6 @@ from seqgen.sequences import Sequences
 
 # Note that the version string must have the following format, otherwise it
 # will not be found by the version() function in ../setup.py
-__version__ = '1.0.12'
+__version__ = '1.0.13'
 
 __all__ = ['Sequences']
diff --git a/seqgen/sequences.py b/seqgen/sequences.py
index b57ba6d..5c3a12a 100644
--- a/seqgen/sequences.py
+++ b/seqgen/sequences.py
@@ -28,11 +28,9 @@ class Sequences(object):
         'description',
         'id',
         'id prefix',
-        'from name',
+        'from id',
         'length',
         'mutation rate',
-        'name',
-        'name count',
         'random aa',
         'random nt',
         'ratchet',
@@ -43,7 +41,7 @@ class Sequences(object):
     }
     LEGAL_SPEC_SECTION_KEYS = {
         'alphabet',
-        'from name',
+        'from id',
         'length',
         'mutation rate',
         'random aa',
@@ -58,6 +56,7 @@ class Sequences(object):
         self._defaultIdPrefix = defaultIdPrefix or self.DEFAULT_ID_PREFIX
         self._readSpecification(spec)
         self._idPrefixCount = {}
+        self._sequences = {}
 
     def _readSpecification(self, spec):
         """
@@ -87,28 +86,6 @@ class Sequences(object):
         self._sequenceSpecs = list(map(self._expandSpec, sequenceSpecs))
         self._checkKeys()
         self._checkValid()
-        self._names = {}
-
-    def _nameCount(self, spec):
-        """
-        Get the name count for a spec
-
-        @param spec: A C{dict} with keys/values specifying a sequence.
-        @return: A 0-based integer count of the sequence index that should be
-            named or C{0} if there is no 'name count' directive in C{spec}.
-        """
-        nSequences = spec.get('count', 1)
-        try:
-            nameCount = spec['name count']
-        except KeyError:
-            return 0
-        else:
-            if nameCount == 'first':
-                return 0
-            elif nameCount == 'last':
-                return nSequences - 1
-            else:
-                return nameCount - 1
 
     def _checkValid(self):
         """
@@ -118,7 +95,7 @@ class Sequences(object):
             to be produced.
         @raise ValueError: If any problem is found.
         """
-        names = set()
+        ids = set()
         for specCount, spec in enumerate(self._sequenceSpecs, start=1):
             if spec.get('ratchet'):
                 nSequences = spec.get('count', 1)
@@ -134,24 +111,27 @@ class Sequences(object):
 
             nSequences = spec.get('count', 1)
 
-            nameCount = self._nameCount(spec)
-            if nameCount is not None and nameCount >= nSequences:
-                raise ValueError(
-                    'Sequence specification %d will only produce %d '
-                    'sequence%s but has a (too high) name count value of %d.' %
-                    (specCount, nSequences, '' if nSequences == 1 else 's',
-                     nameCount + 1))
-
             try:
-                name = spec['name']
+                id_ = spec['id']
             except KeyError:
                 pass
             else:
-                if name in names:
-                    raise ValueError("Name '%s' is duplicated in the JSON "
-                                     "specification." % name)
-                else:
-                    names.add(name)
+                # If an id is given, the number of sequences requested must be
+                # one.
+                if nSequences != 1:
+                    raise ValueError(
+                        "Sequence specification %d with id '%s' has a count "
+                        "of %d. If you want to specify a sequence with an "
+                        "id, the count must be 1. To specify multiple "
+                        "sequences with an id prefix, use 'id prefix'." %
+                        (specCount, id_, nSequences))
+
+                if id_ in ids:
+                    raise ValueError(
+                        "Sequence specification %d has an id (%s) that has "
+                        "already been used." % (specCount, id_))
+
+                ids.add(id_)
 
     def _checkKeys(self):
         """
@@ -239,28 +219,28 @@ class Sequences(object):
             read = Read(None, previousRead.sequence)
             alphabet = previousRead.alphabet
 
-        elif 'from name' in spec:
-            name = spec['from name']
+        elif 'from id' in spec:
+            fromId = spec['from id']
             try:
-                namedRead = self._names[name]
+                fromRead = self._sequences[fromId]
             except KeyError:
-                raise ValueError("Sequence section refers to name '%s' of "
-                                 "non-existent other sequence." % name)
+                raise ValueError("Sequence section refers to the id '%s' of "
+                                 "non-existent other sequence." % fromId)
             else:
                 # The start offset in the spec is 1-based. Convert to 0-based.
                 index = int(spec.get('start', 1)) - 1
                 # Use the given length (if any) else the length of the
                 # named read.
-                length = spec.get('length', len(namedRead))
-                sequence = namedRead.sequence[index:index + length]
-                alphabet = namedRead.alphabet
+                length = spec.get('length', len(fromRead))
+                sequence = fromRead.sequence[index:index + length]
+                alphabet = fromRead.alphabet
 
                 if len(sequence) != length:
                     raise ValueError(
-                        "Sequence specification refers to sequence name '%s', "
-                        "starting at index %d with length %d, but '%s' "
-                        "is not long enough to support that." %
-                        (name, index + 1, length, name))
+                        "Sequence specification refers to sequence id '%s', "
+                        "starting at index %d with length %d, but sequence "
+                        "'%s' is not long enough to support that." %
+                        (fromId, index + 1, length, fromId))
 
                 read = Read(None, sequence)
 
@@ -330,7 +310,6 @@ class Sequences(object):
         alphabet = None
         previousRead = None
         nSequences = spec.get('count', 1)
-        nameCount = self._nameCount(spec)
 
         for count in range(nSequences):
             id_ = None
@@ -356,16 +335,6 @@ class Sequences(object):
                     prefixCount = self._idPrefixCount.setdefault(prefix, 0) + 1
                     self._idPrefixCount[prefix] += 1
                     id_ = '%s%d' % (prefix, prefixCount)
-                else:
-                    # If an id is given, the number of sequences requested must
-                    # be one.
-                    if nSequences != 1:
-                        raise ValueError(
-                            "Sequence with id '%s' has a count of %d. If you "
-                            "want to specify one sequence with an id, the "
-                            "count must be 1. To specify multiple sequences "
-                            "with an id prefix, use 'id prefix'." %
-                            (id_, nSequences))
 
             try:
                 id_ = id_ + ' ' + spec['description']
@@ -375,10 +344,11 @@ class Sequences(object):
             read = Read(id_, sequence)
             read.alphabet = alphabet
 
-            # Keep a reference to this result if it is named and this is the
-            # sequence (by count) we want to name.
-            if 'name' in spec and nameCount == count:
-                self._names[spec['name']] = read
+            if id_ in self._sequenceSpecs:
+                raise ValueError(
+                    "Sequence id '%s' has already been used." % id_)
+            else:
+                self._sequences[id_] = read
 
             if not spec.get('skip'):
                 yield read


Test patch:
diff --git a/test/testSequences.py b/test/testSequences.py
index a57de94..b96fd97 100644
--- a/test/testSequences.py
+++ b/test/testSequences.py
@@ -50,13 +50,14 @@ class TestSequences(TestCase):
                           spec='filename')
 
     @patch(open_, new_callable=mock_open,
-           read_data='[{"name": "a"}, {"name": "a"}]')
-    def testDuplicatedName(self, mock):
+           read_data='[{"id": "a"}, {"id": "a"}]')
+    def testDuplicatedId(self, mock):
         """
-        If a duplicate sequence name is present in the JSON, a ValueError
+        If a duplicate sequence id is present in the JSON, a ValueError
         must be raised.
         """
-        error = "^Name 'a' is duplicated in the JSON specification\\.$"
+        error = ("^Sequence specification 2 has an id \\(a\\) that has "
+                 "already been used\\.$")
         assertRaisesRegex(self, ValueError, error, Sequences, spec='filename')
 
     def testNoSequences(self):
@@ -66,18 +67,6 @@ class TestSequences(TestCase):
         s = Sequences(StringIO('[]'))
         self.assertEqual([], list(s))
 
-    def testOneSequenceNameOnly(self):
-        """
-        If only one sequence is specified, and only by name, one sequence
-        should be created, it should have the default length, the expected
-        id, and it should be entirely composed of nucleotides.
-        """
-        s = Sequences(StringIO('[{"name": "a"}]'))
-        (read,) = list(s)
-        self.assertEqual(Sequences.DEFAULT_ID_PREFIX + '1', read.id)
-        self.assertEqual(Sequences.DEFAULT_LENGTH, len(read.sequence))
-        self.assertEqual(set(), set(read.sequence) - set('ACGT'))
-
     def testOneSequenceIdOnly(self):
         """
         If only one sequence is specified, and only by id, one sequence
@@ -128,19 +117,19 @@ class TestSequences(TestCase):
         If only one sequence is specified with an id, a ValueError must be
         raised if its count is greater than one.
         """
-        s = Sequences(StringIO('''{
+        spec = StringIO('''{
             "sequences": [
                 {
                     "id": "the-id",
                     "count": 6
                 }
             ]
-        }'''))
-        error = ("^Sequence with id 'the-id' has a count of 6\\. If you want "
-                 "to specify one sequence with an id, the count must be 1\\. "
-                 "To specify multiple sequences with an id prefix, use "
-                 "'id prefix'\\.$")
-        assertRaisesRegex(self, ValueError, error, list, s)
+        }''')
+        error = ("^Sequence specification 1 with id 'the-id' has a count of "
+                 "6\\. If you want to specify a sequence with an id, the "
+                 "count must be 1\\. To specify multiple sequences with an id "
+                 "prefix, use 'id prefix'\\.$")
+        assertRaisesRegex(self, ValueError, error, Sequences, spec)
 
     def testRatchetWithNoCount(self):
         """
@@ -196,7 +185,7 @@ class TestSequences(TestCase):
         If an unknown key is given in a sequence specification section, a
         ValueError must be raised.
         """
-        for key in 'id', 'id prefix', 'name':
+        for key in 'id', 'id prefix', 'xxx':
             error = ("^Section 1 of sequence specification 1 contains an "
                      "unknown key: %s\\.$" % key)
             assertRaisesRegex(self, ValueError, error, Sequences, StringIO('''{
@@ -283,23 +272,23 @@ class TestSequences(TestCase):
         the default length, the expected id, and it should be entirely
         composed of nucleotides.
         """
-        s = Sequences(StringIO('[{"name": "a", "random aa": true}]'))
+        s = Sequences(StringIO('[{"random aa": true}]'))
         (read,) = list(s)
         self.assertEqual(Sequences.DEFAULT_ID_PREFIX + '1', read.id)
         self.assertEqual(Sequences.DEFAULT_LENGTH, len(read.sequence))
         self.assertEqual(set(), set(read.sequence) - set(AA_LETTERS))
 
-    def testTwoSequencesSecondFromName(self):
+    def testTwoSequencesSecondFromId(self):
         """
-        If only one sequence is specified by name and a second refers to it
-        by name, the second sequence should be the same as the first.
+        If only one sequence is given an id and a second refers to it
+        by that id, the second sequence should be the same as the first.
         """
         s = Sequences(StringIO('''[
             {
-                "name": "a"
+                "id": "a"
             },
             {
-                "from name": "a"
+                "from id": "a"
             }
         ]'''))
         (read1, read2) = list(s)
@@ -544,7 +533,7 @@ class TestSequences(TestCase):
         (read,) = list(s)
         self.assertEqual(50, len(read.sequence))
 
-    def testSectionWithNameReference(self):
+    def testSectionWithIdReference(self):
         """
         A sequence must be able to be built up from sections, with two
         sections given by length.
@@ -552,13 +541,13 @@ class TestSequences(TestCase):
         s = Sequences(StringIO('''{
             "sequences": [
                 {
-                    "name": "xxx",
+                    "id": "xxx",
                     "sequence": "ACCGT"
                 },
                 {
                     "sections": [
                         {
-                            "from name": "xxx"
+                            "from id": "xxx"
                         },
                         {
                             "length": 10
@@ -571,76 +560,76 @@ class TestSequences(TestCase):
         self.assertEqual(15, len(read2.sequence))
         self.assertTrue(read2.sequence.startswith('ACCGT'))
 
-    def testSectionWithUnknownNameReference(self):
+    def testSectionWithUnknownIdReference(self):
         """
         If a sequence is built up from sections and a referred to sequence
-        does not exist, a ValueError must be raised.
+        id does not exist, a ValueError must be raised.
         """
         s = Sequences(StringIO('''{
             "sequences": [
                 {
                     "sections": [
                         {
-                            "from name": "xxx"
+                            "from id": "xxx"
                         }
                     ]
                 }
             ]
         }'''))
-        error = ("^Sequence section refers to name 'xxx' of "
+        error = ("^Sequence section refers to the id 'xxx' of "
                  "non-existent other sequence\\.$")
         assertRaisesRegex(self, ValueError, error, list, s)
 
-    def testSectionWithNameReferenceTooShort(self):
+    def testSectionWithIdReferenceTooShort(self):
         """
-        If a sequence is built up from sections and a referred to sequence
+        If a sequence is built up from sections and a referred-to sequence
         is too short for the desired length, a ValueError must be raised.
         """
         s = Sequences(StringIO('''{
             "sequences": [
                 {
-                    "name": "xxx",
+                    "id": "xxx",
                     "sequence": "ACCGT"
                 },
                 {
                     "sections": [
                         {
-                            "from name": "xxx",
+                            "from id": "xxx",
                             "length": 10
                         }
                     ]
                 }
             ]
         }'''))
-        error = ("^Sequence specification refers to sequence name 'xxx', "
-                 "starting at index 1 with length 10, but 'xxx' is not long "
-                 "enough to support that\\.$")
+        error = ("^Sequence specification refers to sequence id 'xxx', "
+                 "starting at index 1 with length 10, but sequence 'xxx' is "
+                 "not long enough to support that\\.$")
         assertRaisesRegex(self, ValueError, error, list, s)
 
     def testNamedRecombinant(self):
         """
-        It must be possible to build up and name a recombinant.
+        It must be possible to build up and give an id to a recombinant.
         """
         s = Sequences(StringIO('''{
             "sequences": [
                 {
-                    "name": "xxx",
+                    "id": "xxx",
                     "sequence": "ACCA"
                 },
                 {
-                    "name": "yyy",
+                    "id": "yyy",
                     "sequence": "GGTT"
                 },
                 {
                     "id": "recombinant",
                     "sections": [
                         {
-                            "from name": "xxx",
+                            "from id": "xxx",
                             "start": 1,
                             "length": 3
                         },
                         {
-                            "from name": "yyy",
+                            "from id": "yyy",
                             "start": 2,
                             "length": 2
                         }
@@ -655,26 +644,26 @@ class TestSequences(TestCase):
     def testRecombinantFromFullOtherSequences(self):
         """
         It must be possible to build up a recombinant that is composed of two
-        other sequences by only giving the names of the other sequences.
+        other sequences by only giving the ids of the other sequences.
         """
         s = Sequences(StringIO('''{
             "sequences": [
                 {
-                    "name": "xxx",
+                    "id": "xxx",
                     "sequence": "ACCA"
                 },
                 {
-                    "name": "yyy",
+                    "id": "yyy",
                     "sequence": "GGTT"
                 },
                 {
                     "id": "recombinant",
                     "sections": [
                         {
-                            "from name": "xxx"
+                            "from id": "xxx"
                         },
                         {
-                            "from name": "yyy"
+                            "from id": "yyy"
                         }
                     ]
                 }
@@ -709,13 +698,13 @@ class TestSequences(TestCase):
         s = Sequences(StringIO('''{
             "sequences": [
                 {
-                    "name": "orig",
+                    "id": "orig",
                     "alphabet": "01",
                     "length": %s
                 },
                 {
                     "count": 2,
-                    "from name": "orig",
+                    "from id": "orig",
                     "mutation rate": 1.0,
                     "ratchet": true
                 }
@@ -737,138 +726,3 @@ class TestSequences(TestCase):
         # The sequences of the original and the second mutant must be
         # identical.
         self.assertEqual(orig.sequence, mutant2.sequence)
-
-    def testNameCountTooHigh(self):
-        """
-        A name count value that is higher than the number of sequences in
-        a specification must result in a ValueError.
-        """
-        spec = StringIO('''{
-            "sequences": [
-                {
-                    "name": "orig",
-                    "name count": 10,
-                    "count": 2
-                }
-            ]
-        }''')
-        error = ('^Sequence specification 1 will only produce 2 sequences but '
-                 'has a \\(too high\\) name count value of 10\\.$')
-        assertRaisesRegex(self, ValueError, error, Sequences, spec)
-
-    def testNameCountFirst(self):
-        """
-        A name count value of 'first' must result in the first sequence of a
-        series being named.
-        """
-        s = Sequences(StringIO('''{
-            "sequences": [
-                {
-                    "name": "orig",
-                    "name count": "first",
-                    "count": 20,
-                    "mutation rate": 0.1,
-                    "length": 500,
-                    "ratchet": true
-                },
-                {
-                    "id": "copy",
-                    "from name": "orig"
-                }
-            ]
-        }'''))
-        reads = list(s)
-        copySequence = reads[-1].sequence
-        # The final sequence (the copy) must be the same as the first sequence
-        # (and no others).
-        self.assertEqual(reads[0].sequence, copySequence)
-        for i in range(1, 20):
-            self.assertNotEqual(reads[i].sequence, copySequence)
-
-    def testNameCountLast(self):
-        """
-        A name count value of 'last' must result in the last sequence of a
-        series being named.
-        """
-        s = Sequences(StringIO('''{
-            "sequences": [
-                {
-                    "name": "orig",
-                    "name count": "last",
-                    "count": 20,
-                    "mutation rate": 0.1,
-                    "length": 500,
-                    "ratchet": true
-                },
-                {
-                    "id": "copy",
-                    "from name": "orig"
-                }
-            ]
-        }'''))
-        reads = list(s)
-        copySequence = reads[-1].sequence
-        # The final sequence (the copy) must be the same as the second last
-        # (i.e., the final mutant) sequence (and no others).
-        self.assertEqual(reads[-2].sequence, copySequence)
-        for i in range(19):
-            self.assertNotEqual(reads[i].sequence, copySequence)
-
-    def testNameCountOne(self):
-        """
-        A name count value of 1 must result in the first sequence of a
-        series being named.
-        """
-        s = Sequences(StringIO('''{
-            "sequences": [
-                {
-                    "name": "orig",
-                    "name count": 1,
-                    "count": 20,
-                    "mutation rate": 0.1,
-                    "length": 500,
-                    "ratchet": true
-                },
-                {
-                    "id": "copy",
-                    "from name": "orig"
-                }
-            ]
-        }'''))
-        reads = list(s)
-        copySequence = reads[-1].sequence
-        # The final sequence (the copy) must be the same as the first sequence
-        # (and no others).
-        self.assertEqual(reads[0].sequence, copySequence)
-        for i in range(1, 20):
-            self.assertNotEqual(reads[i].sequence, copySequence)
-
-    def testNameCountTwo(self):
-        """
-        A name count value of 2 must result in the second sequence of a
-        series being named.
-        """
-        s = Sequences(StringIO('''{
-            "sequences": [
-                {
-                    "name": "orig",
-                    "name count": 2,
-                    "count": 20,
-                    "mutation rate": 0.1,
-                    "length": 500,
-                    "ratchet": true
-                },
-                {
-                    "id": "copy",
-                    "from name": "orig"
-                }
-            ]
-        }'''))
-        reads = list(s)
-        copySequence = reads[-1].sequence
-        # The final sequence (the copy) must be the same as the second sequence
-        # (and no others).
-        self.assertEqual(reads[1].sequence, copySequence)
-        self.assertNotEqual(reads[0].sequence, copySequence)
-        for i in range(2, 20):
-            self.assertNotEqual(reads[i].sequence, copySequence)