Adding the second VcfParser class: NucleusParser #346

samanvp · 2018-08-24T18:53:27Z

After defining the VCFParser interface and PyVcfParser in a previous PR, here we add
NucleusParser. Following points must be highlighted:

This new parser is hidden behind a flag (use_nucleus=False).
Since we are not still able to create a Wheel file that have the
from_string() method, we use a hack to make Nucleus work: write the whole
file to a local temp file and send it as input to parser.
Currently we don't have any unit tests or integration test. We
manually cheked the sanity of Nucleus output by comparing its output
table with PyVcf output. This must be done in the next step.

arostamianfar

Thanks, Saman. I added some comments. Please also add unit tests just for the proto->class conversion for now. We can add more once the from_string method is available.

arostamianfar · 2018-08-24T19:59:36Z

gcp_variant_transforms/beam_io/vcf_parser.py

 import vcf

 from apache_beam.coders import coders
+from apache_beam.io.filesystems import FileSystems


Please only import the filesystems module (this is required by the styleguide [1], but not followed by Beam, which is why there is sometimes a discrepancy in our code).
[1] https://github.com/google/styleguide/blob/gh-pages/pyguide.md#22-imports

arostamianfar · 2018-08-24T20:00:10Z

gcp_variant_transforms/beam_io/vcf_parser.py

+          break
+    return temp_file_name
+
+  def _store_to_temp_local_file(self, header_lines):


this is not used?

I had this for our final design that we will have from_string(), I though I will just leave it here until we drop the current hack and will have our final implementation.

I will remove it now and add later ...

Now that we have the from_string() I added this back.

arostamianfar · 2018-08-24T20:05:35Z

gcp_variant_transforms/beam_io/vcf_parser.py

+    # This member will be properly initiated in _init_with_header().
+    self._vcf_reader = None
+    # These members will be properly initiated in _extract_header_fields().
+    self._is_info_repeated = {}


Please create a dict of info and format that point to VcfInfo and VcfFormatInfo instead; that is more future-proof (e.g. if we want to also check the description or type for another purpose, we could just reuse those dicts later on without introducing more specialized dicts). You can keep the is_repeated arg in _convert_list_value as it's a nice abstraction, but you can make a helper function that given a number string would return true/false if the field is repeated.

Done.
I also added two other methods so finding is_repeated would be more clear.

arostamianfar · 2018-08-24T20:06:08Z

gcp_variant_transforms/beam_io/vcf_parser.py

+
+  def _init_with_header(self, header_lines):
+    # This header line is needed by Nucleus.
+    header_lines = ['##fileformat=VCFv4.2'] + header_lines


Please only add this if the first like is not ##fileformat.

good point!

arostamianfar · 2018-08-24T20:06:19Z

gcp_variant_transforms/beam_io/vcf_parser.py

+
+  def _init_with_header(self, header_lines):
+    # This header line is needed by Nucleus.
+    header_lines = ['##fileformat=VCFv4.2'] + header_lines


Please make this a constant.

Now the question is, how about the const string ##fileformat which we use in the if condition? It seems a bit too redundant if we define that as another constant?

Let me know what do you think?

What do you think of this suggestion: define FILE_FORMAT_HEADER_TEMPLATE='##fileformat=VCF{VERSION}'.
You can then do startswith(FILE_FORMAT_HEADER_TEMPLATE.format(VERSION='') ...if it doesn't match, you can add FILE_FORMAT_HEADER_TEMPLATE.format('v4.0'). I think it's ok to hard-code the 'v.4.0' for simplicity.

arostamianfar · 2018-08-24T20:33:03Z

gcp_variant_transforms/beam_io/vcf_parser.py

+                      self._file_name, data_line, str(e))
+      return MalformedVcfRecord(self._file_name, data_line, str(e))
+
+  def _convert_to_variant(self, record):


s/record/variant_proto (we used 'record' before b/c that was the PyVCF naming).

arostamianfar · 2018-08-24T20:33:27Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      return MalformedVcfRecord(self._file_name, data_line, str(e))
+
+  def _convert_to_variant(self, record):
+    # type: (nucleus_proto.Variant) -> Variant


The type looks wrong as there is no "nucleus_proto" module?

In my 'proper' implementation (using the from_string() method), we need:

from nucleus.protos import variants_pb2 as nucleus_proto

to initialize the Nucleus object (their signature has changed from the current version).
Here we don't need to import that module.

arostamianfar · 2018-08-24T20:34:07Z

gcp_variant_transforms/beam_io/vcf_parser.py

+        info=self._get_variant_info(record),
+        calls=self._get_variant_calls(record))
+
+  def _get_variant_info(self, record):


s/record/variant_proto

arostamianfar · 2018-08-24T20:35:36Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      if not sample.genotype:
+        call.genotype.append(MISSING_GENOTYPE_VALUE)
+      else:
+        for v in sample.genotype:


I think you can just do call.genotype = sample.genotype (or if it's really not a list, just cast to a list like list(sample.genotype))

We need to convert it to list otherwise it fails.

arostamianfar · 2018-08-24T20:39:34Z

gcp_variant_transforms/beam_io/vcfio.py

@@ -186,7 +186,8 @@ def __init__(self,
               compression_type=CompressionTypes.AUTO,  # type: str
               buffer_size=DEFAULT_VCF_READ_BUFFER_SIZE,  # type: int
               validate=True,  # type: bool
-               allow_malformed_records=False  # type: bool
+               allow_malformed_records=False,  # type: bool
+               use_nucleus=False  # type: bool


it's more future proof to make an enum for this like VcfParserType and use that here (in case we decide to add a third parser). There is even a 'simpler' way of just using the class string and let python actually construct it (so you dont even need an if statement) but that may be less readable.

Do you think we need a more complicated approach given that we are pretty sure there is not going to be another parser anytime soon?

If you think still we better use the enum let me know, I will do it in the next round of comments.

I think we should. It's very reasonable to have other types of parsers in the future (e.g. we're thinking about a bgzip parser; which may end up being a layer on top of Nuclues). In general, it's a good software engineering practice to not use bools for things that can possibly have more than two values.

samanvp

except for a strange pylint message (which I am looking into it) this PR is ready for review.

Also I have started working on unit tests in a separate PR, if you prefer I can add them to this one.

Thanks.

samanvp · 2018-08-24T21:20:03Z

gcp_variant_transforms/beam_io/vcf_parser.py

 import vcf

 from apache_beam.coders import coders
+from apache_beam.io.filesystems import FileSystems


samanvp · 2018-08-24T21:22:59Z

gcp_variant_transforms/beam_io/vcf_parser.py

+    self._temp_local_file = self._write_to_local_file(file_name)
+
+  def _write_to_local_file(self, remote_file_name):
+    (temp_file, temp_file_name) = tempfile.mkstemp(text=True)


samanvp · 2018-08-24T21:24:29Z

gcp_variant_transforms/beam_io/vcf_parser.py

+          break
+    return temp_file_name
+
+  def _store_to_temp_local_file(self, header_lines):


I had this for our final design that we will have from_string(), I though I will just leave it here until we drop the current hack and will have our final implementation.

I will remove it now and add later ...

samanvp · 2018-08-24T21:27:13Z

gcp_variant_transforms/beam_io/vcf_parser.py

+
+  def _init_with_header(self, header_lines):
+    # This header line is needed by Nucleus.
+    header_lines = ['##fileformat=VCFv4.2'] + header_lines


good point!

samanvp · 2018-08-24T21:32:41Z

gcp_variant_transforms/beam_io/vcf_parser.py

+
+  def _init_with_header(self, header_lines):
+    # This header line is needed by Nucleus.
+    header_lines = ['##fileformat=VCFv4.2'] + header_lines


Now the question is, how about the const string ##fileformat which we use in the if condition? It seems a bit too redundant if we define that as another constant?

Let me know what do you think?

samanvp · 2018-08-27T17:23:01Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      return None
+    if len(output_list) == 1:
+      return output_list[0]
+    raise ValueError('a not repeated field has more than 1 value')


I believe, regardless of what the Nucleus does, we need to do the right thing and fail (or at least report this problem) instead of just dropping unexpected values.
If you think failing is not a good idea, we could change this to a warning. Let me know what you think.

samanvp · 2018-08-27T18:47:11Z

gcp_variant_transforms/beam_io/vcf_parser.py

+    # This member will be properly initiated in _init_with_header().
+    self._vcf_reader = None
+    # These members will be properly initiated in _extract_header_fields().
+    self._is_info_repeated = {}


Done.
I also added two other methods so finding is_repeated would be more clear.

samanvp · 2018-08-31T13:33:41Z

gcp_variant_transforms/beam_io/vcf_parser.py

+          break
+    return temp_file_name
+
+  def _store_to_temp_local_file(self, header_lines):


Now that we have the from_string() I added this back.

samanvp · 2018-08-31T13:35:12Z

gcp_variant_transforms/beam_io/vcf_parser.py

-        alternate_bases=variant_proto.alternate_bases,
-        names=map(str, variant_proto.names),
+        alternate_bases=list(variant_proto.alternate_bases),
+        names=variant_proto.names[0].split(';') if variant_proto.names else [],


We should report this as a bug to Nucleus team.

samanvp · 2018-08-31T13:36:42Z

gcp_variant_transforms/beam_io/vcf_parser.py

-      if phaseset_from_format or call_proto.is_phased:
-        call.phaseset = (phaseset_from_format if phaseset_from_format
-                         else DEFAULT_PHASESET_VALUE)
+      if call_proto.is_phased:


I modified this if statement, I think this is more straightforward. Let me know what you think.

arostamianfar

Thanks, Saman. This looks much better! Please also submit unit tests together with this PR as we want to avoid having untested code in our repo at any point; may also be easier to catch/fix strange errors.
P.S. let's sync offline about how to do setup.py properly so that the unit tests actually pass.

arostamianfar · 2018-08-31T16:14:04Z

gcp_variant_transforms/beam_io/vcf_parser.py

+import os
+import tempfile
+
+from nucleus.io.python import vcf_reader as nucleus


s/nucleus/nucleus_vcf_reader so that it doesn't conflict with import nucleus itself (in case someone wants to do that in the future).

arostamianfar · 2018-08-31T16:14:43Z

gcp_variant_transforms/beam_io/vcf_parser.py

+import tempfile
+
+from nucleus.io.python import vcf_reader as nucleus
+from nucleus.protos import variants_pb2 as nucleus_proto


Please use variants_pb2 here. The one above is fine as you like to distinguish between pyvcf and nucleus parsers, but you should leave this one as is (e.g. it's easier for searching).

arostamianfar · 2018-08-31T16:18:45Z

gcp_variant_transforms/beam_io/vcf_parser.py

@@ -41,7 +46,7 @@
 DEFAULT_PHASESET_VALUE = '*'  # Default phaseset value if call is phased, but
                              # no 'PS' is present.
 MISSING_GENOTYPE_VALUE = -1  # Genotype to use when '.' is used in GT field.
-
+FIRST_HEADER_LINE = '##fileformat=VCFv4.2\n'


To clarify: do all header_lines passed via _init_with_header include a \n?

Yes, originally I did not have \n and I noticed header was missing AC info field. When I further looked at the problem I found out the top two lines are concatenated as one line (and Nucleus actually did not show any warning or error message).

arostamianfar · 2018-08-31T16:19:28Z

gcp_variant_transforms/beam_io/vcf_parser.py

@@ -41,7 +46,7 @@
 DEFAULT_PHASESET_VALUE = '*'  # Default phaseset value if call is phased, but
                              # no 'PS' is present.
 MISSING_GENOTYPE_VALUE = -1  # Genotype to use when '.' is used in GT field.
-
+FIRST_HEADER_LINE = '##fileformat=VCFv4.2\n'


I think we should use 4.0 as the 'least common denominator'. Not sure if it matters for the parser though.

arostamianfar · 2018-08-31T16:21:07Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      return MalformedVcfRecord(self._file_name, data_line, str(e))
+
+  def _convert_to_variant(self, variant_proto):
+    # type: (nucleus.protos.variants_pb2.Variant) -> Variant


nit: just variants_pb2.Variant is fine here :)

arostamianfar · 2018-08-31T16:34:40Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      # This is a temporary solution until from_string will be fixed.
+      record = next(self._vcf_reader)
+      return self._convert_to_variant(record)
+    except (LookupError, ValueError) as e:


I actually don't see LookupError in their code. Looks like they only throw ValueError in their from_string signature.

arostamianfar · 2018-08-31T16:37:11Z

gcp_variant_transforms/beam_io/vcf_parser.py

+                         if record.reference_bases != MISSING_FIELD_VALUE
+                         else None),
+        alternate_bases=(
+            map(str, record.alternate_bases) if record.alternate_bases else []),


This is strange. We should investigate this further later. OK for this PR.

arostamianfar · 2018-08-31T16:37:56Z

gcp_variant_transforms/beam_io/vcf_parser.py

+        alternate_bases=(
+            map(str, record.alternate_bases) if record.alternate_bases else []),
+        names=map(str, record.names) if record.names else [],
+        quality=record.quality,


Right; it seems like they use -1 instead of null for quality field. We should eventually revisit this and special case on that. OK to leave as TODO for this PR.

arostamianfar · 2018-08-31T16:39:13Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      return None
+    if len(output_list) == 1:
+      return output_list[0]
+    raise ValueError('a not repeated field has more than 1 value')


We should only throw an error if allow_malformed_records is true. But let's first see what Nucleus does. OK to just add a TODO for this PR though.

arostamianfar · 2018-08-31T16:40:55Z

gcp_variant_transforms/beam_io/vcf_parser.py

+
+  def _get_variant_calls(self, record):
+    calls = []
+    for sample in record.calls:


I think in this context, it's better to use consistent names within each class rather than across classes. There are some terminologies that are PyVCF specific that we shouldn't leak into Nucleus ;) (e.g. when/if we remove PyVCF).

samanvp

I added all modifications that you asked for. I also duplicated two of the unit tests so we have a minimum level of tests for Nucleus. Please let me know if you think this is enough.

samanvp · 2018-08-31T17:11:15Z

gcp_variant_transforms/beam_io/vcf_parser.py

+import os
+import tempfile
+
+from nucleus.io.python import vcf_reader as nucleus


samanvp · 2018-08-31T17:12:13Z

gcp_variant_transforms/beam_io/vcf_parser.py

+import tempfile
+
+from nucleus.io.python import vcf_reader as nucleus
+from nucleus.protos import variants_pb2 as nucleus_proto


samanvp · 2018-08-31T17:15:10Z

gcp_variant_transforms/beam_io/vcf_parser.py

@@ -41,7 +46,7 @@
 DEFAULT_PHASESET_VALUE = '*'  # Default phaseset value if call is phased, but
                              # no 'PS' is present.
 MISSING_GENOTYPE_VALUE = -1  # Genotype to use when '.' is used in GT field.
-
+FIRST_HEADER_LINE = '##fileformat=VCFv4.2\n'


Yes, originally I did not have \n and I noticed header was missing AC info field. When I further looked at the problem I found out the top two lines are concatenated as one line (and Nucleus actually did not show any warning or error message).

samanvp · 2018-08-31T17:15:46Z

gcp_variant_transforms/beam_io/vcf_parser.py

@@ -41,7 +46,7 @@
 DEFAULT_PHASESET_VALUE = '*'  # Default phaseset value if call is phased, but
                              # no 'PS' is present.
 MISSING_GENOTYPE_VALUE = -1  # Genotype to use when '.' is used in GT field.
-
+FIRST_HEADER_LINE = '##fileformat=VCFv4.2\n'


samanvp · 2018-08-31T17:17:12Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      return MalformedVcfRecord(self._file_name, data_line, str(e))
+
+  def _convert_to_variant(self, variant_proto):
+    # type: (nucleus.protos.variants_pb2.Variant) -> Variant


samanvp · 2018-08-31T17:24:49Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      else:
+        call.genotype = list(call_proto.genotype)
+
+      if call_proto.is_phased:


What is your reasoning for changing it?

Nucleus comment on is_phased field implies that the value of PS field is only relevant when is_phased == True.
I changed this to the old logic.

samanvp · 2018-08-31T17:42:01Z

gcp_variant_transforms/beam_io/vcfio.py

@@ -186,7 +186,8 @@ def __init__(self,
               compression_type=CompressionTypes.AUTO,  # type: str
               buffer_size=DEFAULT_VCF_READ_BUFFER_SIZE,  # type: int
               validate=True,  # type: bool
-               allow_malformed_records=False  # type: bool
+               allow_malformed_records=False,  # type: bool
+               use_nucleus=False  # type: bool


samanvp · 2018-08-31T18:55:25Z

gcp_variant_transforms/beam_io/vcf_parser.py

+      # This is a temporary solution until from_string will be fixed.
+      record = next(self._vcf_reader)
+      return self._convert_to_variant(record)
+    except (LookupError, ValueError) as e:


You are right, last time I quickly checked this test file and I saw they are checking both of those exception. Now that I checked the test_vcf_from_string_raises_on_bad_input I saw they only check the ValueError for from_string() method.

coveralls · 2018-09-10T18:52:10Z

Pull Request Test Coverage Report for Build 1264

90 of 221 (40.72%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-1.7%) to 88.102%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
gcp_variant_transforms/beam_io/vcfio.py	9	12	75.0%
gcp_variant_transforms/beam_io/vcf_parser_test.py	33	36	91.67%
gcp_variant_transforms/beam_io/vcfio_test.py	28	59	47.46%
gcp_variant_transforms/beam_io/vcf_parser.py	20	114	17.54%

Totals
Change from base Build 1247:	-1.7%
Covered Lines:	5887
Relevant Lines:	6682

💛 - Coveralls

arostamianfar

Thanks, Saman! Looks great! Just two nits (one is optional).

arostamianfar · 2018-09-11T16:07:27Z

gcp_variant_transforms/beam_io/vcf_parser.py

+    try:
+      variant_proto = self._vcf_reader.from_string(data_line)
+      return self._convert_to_variant(variant_proto)
+    except (ValueError) as e:


nit: please remove the surrounding brackets around ValueError

arostamianfar · 2018-09-11T16:17:34Z

gcp_variant_transforms/beam_io/vcfio.py

-        self._representative_header_lines,
-        buffer_size=self._buffer_size,
-        skip_header_lines=0)
+    if self._vcf_parser_type == VcfParserType.PYVCF:


nit: Since the signature of the classes is exactly the same, you can do:

vcf_parser_class = None if self._vcf_parser_type == VcfParserType.PYVCF: vcf_parser_class = vcf_parser.PyVcfParser elif self._vcf_parser_type == VcfParserType.NUCLEUS: vcf_parser_class = vcf_parser.NucleusParser else: raise Error record_iterator = vcf_parser_class( file_name, range_tracker, ....)

This has the advantage of not duplicating the code for the arguments, but we may need to change it back if at any point the signature of the subclasses is not the same. So, up to you if you want to change it :)

arostamianfar

LGTM! Please double check that integration tests pass prior to submitting.

as the second class that implements the VcfParser interface. Currently we are trying to solve a few issues related to the nucleus Wheel file.

After defining the VCFParser interface and PyVcfParser in the PR we add NucleusParser. Following points must be considered: * This new parser is hidden behind a flag (use_nucleus=False). * Since we are not still able to create a Wheel file that have the from_string() method, we use a hack to make Nucleus work: write the whole file to a local temp file and send it as input to parser. * Currently we don't have any unit tests or integration test. We manually cheked the sanity of Nucleus output by comparing its output table with PyVcf output. This must be done in the next step.

as proposed by Asha.

After we managed to build a Wheel file based on the latest code base. We also make a few minor changes to make NucleusParser work with Dataflow.

and also move unit tests of Variant and VariantCall classes to vcf_parser_test.py

In order to use NucleusParser with a DataflowRuner the following flag is still needed: --extra_package /dirctory/containing/Nucleus-0.1.0-py2-none-any.whl

In PR googlegenomics#346 we added a temporary solution to install Nucleus using its wheel file. Now that Nucleus has pip install we can remove that hack.

In PR googlegenomics#346 we added a temporary solution to install Nucleus using its wheel file. Nucleus now has pip install so we can remove that hack.

* Adding google-nucleus to the REQUIRED_PACKAGES list In PR #346 we added a temporary solution to install Nucleus using its wheel file. Nucleus now has pip install so we can remove that hack. * Review comments

samanvp requested a review from arostamianfar August 24, 2018 19:00

arostamianfar reviewed Aug 24, 2018

View reviewed changes

samanvp force-pushed the nucleus_parser branch from 67a6340 to 13640a6 Compare August 27, 2018 20:52

samanvp commented Aug 31, 2018

View reviewed changes

arostamianfar reviewed Aug 31, 2018

View reviewed changes

samanvp force-pushed the nucleus_parser branch 2 times, most recently from 5282426 to b845c24 Compare September 5, 2018 20:51

samanvp commented Sep 5, 2018

View reviewed changes

samanvp force-pushed the nucleus_parser branch 3 times, most recently from df52894 to d8d17f3 Compare September 10, 2018 18:42

samanvp force-pushed the nucleus_parser branch from d8d17f3 to 89d9266 Compare September 10, 2018 19:23

arostamianfar reviewed Sep 17, 2018

View reviewed changes

arostamianfar approved these changes Sep 18, 2018

View reviewed changes

samanvp added 9 commits September 18, 2018 12:11

Add NucleusParser class

1e7cef1

as the second class that implements the VcfParser interface. Currently we are trying to solve a few issues related to the nucleus Wheel file.

Avoid failing builds due to missing nucleus module

5494f5b

as proposed by Asha.

First round of comments

fde951c

Adding from_string() to NucleusParser

eee5d4c

After we managed to build a Wheel file based on the latest code base. We also make a few minor changes to make NucleusParser work with Dataflow.

Second round of comments

6ec70c8

Add unit tests for NucleusParser

3fe77bc

and also move unit tests of Variant and VariantCall classes to vcf_parser_test.py

A temporary solution to make the Travis build pass

3dbe5ff

In order to use NucleusParser with a DataflowRuner the following flag is still needed: --extra_package /dirctory/containing/Nucleus-0.1.0-py2-none-any.whl

Next round of comments

5c2fc6f

samanvp force-pushed the nucleus_parser branch from 11bf049 to 5c2fc6f Compare September 18, 2018 16:11

samanvp merged commit befbca5 into googlegenomics:master Sep 18, 2018

samanvp deleted the nucleus_parser branch September 18, 2018 17:45

samanvp restored the nucleus_parser branch September 19, 2018 18:22

samanvp mentioned this pull request Nov 22, 2018

Adding google-nucleus to the REQUIRED_PACKAGES list #416

Merged

samanvp deleted the nucleus_parser branch September 13, 2019 17:56

Adding the second VcfParser class: NucleusParser #346

Adding the second VcfParser class: NucleusParser #346

Conversation

samanvp commented Aug 24, 2018

arostamianfar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samanvp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arostamianfar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samanvp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 10, 2018 • edited Loading

Pull Request Test Coverage Report for Build 1264

💛 - Coveralls

arostamianfar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arostamianfar left a comment

Choose a reason for hiding this comment

coveralls commented Sep 10, 2018 •

edited

Loading