Repository: brentp/cyvcf2 (language: python)

Issue:
variant.FILTER value PASS
variant.FILTER returns None when its value in VCf file is PASS, as stated in the documentation. However, it also returns None when the FILTER value is missing (shown with '.'). This would create conflict if one would like to filter the rows based on the value PASS. Would you consider to change the current behaviour of the function, so that it would return PASS instead of None? 


Discussion / hints:
Distinguishing them is a good idea, but that would break backwards compatibility. For instance my code is:

```
        if variant.FILTER:
            self.add_filters(variant.FILTER)
```

Assuming most people wrote code like that (and not ```is None```) a possible solution would be to return a "falsy" value (ie evaluates to false) that is different than None. Blank string ('') or 0 would work.

The VCF spec says:

>  ‘0’ is reserved and should not be used as a filter String.  If filters have not been applied,then this field should be set to the missing value

Another option would be to pass in a flag specifying behaviour. But +1 vote for not breaking backward compatability please.

I'd be open to a pull-request that adds the attribute `variant.FILTERS` that returns a list.
Is it not possible to just return a `str` of what is in the `FILTER` column? Then the user can decide what they want to do with it? 
That would break backwards compatability.  My suggestion was a hack to change it in a way that will probably not break things but thinking about it more, a library should never ever have the possibility of altering behavior of those that depend on it.

Brent's suggestion to make a new API is the right thing to do IMO
It's fine to break backwards compatibility though - provided you bump the major version.   
People generally have quite different requirements for how they want to interpret the `FILTER` column. The most flexible thing to do from a library point-of-view seems to be providing the raw contents and letting the user decide how to interpret it.
I'm curious if this is an actual issue that people are facing in practice.
The only issue I see that people could have would be if they wanted to distinguish between `PASS` and `.` - as @sbilge mentioned in the initial issue comment.  

Fix patch:
diff --git a/cyvcf2/cyvcf2.pyx b/cyvcf2/cyvcf2.pyx
index c4d4ebf..a9be87a 100644
--- a/cyvcf2/cyvcf2.pyx
+++ b/cyvcf2/cyvcf2.pyx
@@ -1995,6 +1995,16 @@ cdef class Variant(object):
             if ret != 0:
                 raise Exception("not able to set filter: %s", filters)
 
+    property FILTERS:
+        """the FILTER values as a list from the VCF field.
+
+        a value '.' in the VCF will return an empty list for this property
+        """
+        def __get__(self):
+            cdef int i
+            cdef int n = self.b.d.n_flt
+            return [from_bytes(bcf_hdr_int2id(self.vcf.hdr, BCF_DT_ID, self.b.d.flt[i])) for i in range(n)]
+
     property QUAL:
         "the float value of QUAL from the VCF field."
         def __get__(self):


Test patch:
diff --git a/cyvcf2/tests/test_reader.py b/cyvcf2/tests/test_reader.py
index c11223e..e44717c 100644
--- a/cyvcf2/tests/test_reader.py
+++ b/cyvcf2/tests/test_reader.py
@@ -321,6 +321,36 @@ def test_writer():
     with open(VCF_PATH) as fp:
         run_writer(Writer(fp, v), f, rec)
 
+def test_filters():
+    v = VCF(VCF_PATH)
+    f = tempfile.mktemp(suffix=".vcf")
+    atexit.register(os.unlink, f)
+    rec = next(v)
+
+    writer = Writer(f, v)
+
+    rec.INFO["AC"] = "3"
+    rec.FILTER = ["LowQual"]
+    writer.write_record(rec)
+
+    rec.FILTER = ["LowQual", "VQSRTrancheSNP99.90to100.00"]
+    writer.write_record(rec)
+
+    rec.FILTER = "PASS"
+    writer.write_record(rec)
+
+    rec.FILTER = []
+    writer.write_record(rec)
+
+    writer.close()
+
+    expected_filter = ["LowQual", "LowQual;VQSRTrancheSNP99.90to100.00", None, None]
+    expected_filters = [["LowQual"], ["LowQual", "VQSRTrancheSNP99.90to100.00"], ["PASS"], []]
+
+    for i, variant in enumerate(VCF(f)):
+        assert variant.FILTER == expected_filter[i], (variant.FILTER, expected_filter[i])
+        assert variant.FILTERS == expected_filters[i], (variant.FILTERS, expected_filters[i])
+
 def test_add_info_to_header():
     v = VCF(VCF_PATH)
     v.add_info_to_header({'ID': 'abcdefg', 'Description': 'abcdefg',