From 4defeca232712526a7f6f19f803fae24523602d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Arnoux?= <arnoux.jeromepj@gmail.com>
Date: Mon, 23 Oct 2023 11:04:46 +0200
Subject: [PATCH] resolve some review comment

---
 ppanggolin/annotate/annotate.py      |  7 ++++---
 ppanggolin/formats/writeSequences.py |  3 +--
 ppanggolin/genome.py                 | 20 ++++++--------------
 ppanggolin/projection/projection.py  |  2 +-
 ppanggolin/region.py                 |  4 ++--
 5 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/ppanggolin/annotate/annotate.py b/ppanggolin/annotate/annotate.py
index 871c9e76..9f25fe4e 100644
--- a/ppanggolin/annotate/annotate.py
+++ b/ppanggolin/annotate/annotate.py
@@ -239,7 +239,8 @@ def read_org_gbff(organism_name: str, gbff_file_path: Path, circular_contigs: Li
             sequence += line[10:].replace(" ", "").strip().upper()
             line = lines.pop()
         
-        contig.add_contig_length(len(sequence))
+        if contig.length != len(sequence):
+            raise ValueError("The contig lenght defined is different than the sequence length")
 
         # get each gene's sequence.
         for gene in contig.genes:
@@ -371,8 +372,8 @@ def get_id_attribute(attributes_dict: dict) -> str:
     if has_fasta and fasta_string != "":
         contig_sequences = read_fasta(org, fasta_string.split('\n'))  # _ is total contig length
         for contig in org.contigs:
-
-            contig.add_contig_length(len(contig_sequences[contig.name]))
+            if contig.length != len(contig_sequences[contig.name]):
+                raise ValueError("The contig lenght defined is different than the sequence length")
 
             for gene in contig.genes:
                 gene.add_sequence(get_dna_sequence(contig_sequences[contig.name], gene))
diff --git a/ppanggolin/formats/writeSequences.py b/ppanggolin/formats/writeSequences.py
index 30797321..acbface1 100644
--- a/ppanggolin/formats/writeSequences.py
+++ b/ppanggolin/formats/writeSequences.py
@@ -14,10 +14,9 @@
 # local libraries
 from ppanggolin.pangenome import Pangenome
 from ppanggolin.geneFamily import GeneFamily
-from ppanggolin.genome import Gene
+from ppanggolin.genome import Gene, Organism
 from ppanggolin.utils import write_compressed_or_not, mk_outdir, read_compressed_or_not, restricted_float, detect_filetype
 from ppanggolin.formats.readBinaries import check_pangenome_info, get_gene_sequences_from_file
-from ppanggolin.genome import Organism
 
 module_regex = re.compile(r'^module_[0-9]+')
 poss_values = ['all', 'persistent', 'shell', 'cloud', 'rgp', 'softcore', 'core', module_regex]
diff --git a/ppanggolin/genome.py b/ppanggolin/genome.py
index fd5d1243..743a4eb4 100644
--- a/ppanggolin/genome.py
+++ b/ppanggolin/genome.py
@@ -359,7 +359,12 @@ def length(self, contig_len: int):
             raise TypeError("Contig length is expected to be an integer")
         if contig_len < 0:
             raise ValueError("Contig length must be positive")
-        self._length = contig_len
+
+        if self.length is None:
+            self._length = contig_len
+        elif self.length != contig_length:
+            raise ValueError('Attempting to define a contig length different from the previously defined value.')
+        
 
     def __len__(self):
         return self.length
@@ -525,19 +530,6 @@ def number_of_rnas(self) -> int:
         """
         return len(self._rna_getter)
 
-    def add_contig_length(self, contig_length: int):
-        """
-        Add contig length to Contig object.
-
-        :param contig_length: Length of the contig.
-        :raises ValueError: If trying to define a contig length different than previously defined.
-        """
-        if self.length is None:
-            self.length = contig_length
-
-        elif self.length != contig_length:
-            raise ValueError('Attempting to define a contig length different from the previously defined value.')
-
 
 class Organism(MetaFeatures):
     """
diff --git a/ppanggolin/projection/projection.py b/ppanggolin/projection/projection.py
index c3ff7133..3bd6a42a 100644
--- a/ppanggolin/projection/projection.py
+++ b/ppanggolin/projection/projection.py
@@ -1049,7 +1049,7 @@ def predict_spot_in_one_organism(
 
     input_org_spots = {spot for spots in input_rgp_to_spots.values()
                  for spot in spots }
-    new_spots = {spot for spot in input_org_spots if type(spot) == NewSpot}
+    new_spots = {spot for spot in input_org_spots if isinstance(spot, NewSpot)}
 
 
     logging.getLogger('PPanGGOLiN').debug(
diff --git a/ppanggolin/region.py b/ppanggolin/region.py
index 58700a1e..da434858 100644
--- a/ppanggolin/region.py
+++ b/ppanggolin/region.py
@@ -245,7 +245,7 @@ def contig(self) -> Contig:
         return self.starter.contig
     
     @property
-    def start(self) -> Contig:
+    def start(self) -> int:
         """ 
         Get the starter start link to RGP
 
@@ -254,7 +254,7 @@ def start(self) -> Contig:
         return self.starter.start
     
     @property
-    def stop(self) -> Contig:
+    def stop(self) -> int:
         """ 
         Get the stopper stop link to RGP