Created Cohort Viewer that shows Excluded Patients #85
Conversation
There was a problem hiding this comment.
Hi @lnrekerle
looks good, thanks!
I fixed one bug in the code that searched for overlap between protein coordinates and a protein feature. I added few new methods to Region to do that.
Otherwise, there is one thing that I'd like to point out (see also the comment below). You should use type annotations to write your code. When you look into Region, the start and end properties return int. Not an int or None. Therefore, testing if the coordinate is None is redundant and wrong.
This is the point of using the type annotations. Please work on adding them into your repertoire.
…anscriptAnnotation`, and validate inputs in `TranscriptAnnotation`.
There was a problem hiding this comment.
Hi @lnrekerle looks good, thank you!
While looking at the ProteinMetadata.get_features_variant_overlaps(), I went ahead and changed the signature to take a Region instead of a pair of ints. Thanks to this we have few things less to worry about in the downstream code.
First, we do not have to worry about an invalid state in the method where start comes after end. We would have to check this in the function to be sure but now we don't have to, because coordinate "sanity" is one of Region's invariants.
Next, we don't have to worry about coordinate system in the downstream code, e.g. in predicates. Thanks to using Region, we can be sure that we're comparing apples with apples, when e.g. checking the values in ProtFeatureTypePredicate or ProtFeaturePredicate predicates. In other words, we are certain that the coordinates are in the same coordinate system and we are not prone to off-by-one bugs, a very frequent bugs in bioinformatics due to mishandling of the coordinate systems.
With these, I think we're good to merge, thanks a lot!
No description provided.