186 generalise rankcomparisongenerator class #192
Conversation
yaseminbridges
requested review from
julesjacobsen,
souzadevinicius and
matentzn
| RankComparisonGenerator( | ||
| prioritisation_data.variant_prioritisation.ranks | ||
| ).generate_variant_output(f"{prioritisation_data.variant_prioritisation.results_dir.name}") | ||
| RankComparisonGenerator(prioritisation_data.variant_prioritisation.ranks).generate_output( |
There was a problem hiding this comment.
@julesjacobsen I am sure this is one of the things where you believe Python has gone wrong over a beautiful Java OO design..
There was a problem hiding this comment.
Nope. The previous implementation was all useless repetition. No one wants that.
There was a problem hiding this comment.
I meant prioritisation_data.variant_prioritisation.ranks without encapsulation!
| RankComparisonGenerator(prioritisation_data.gene_prioritisation.ranks).generate_gene_output( | ||
| f"{prioritisation_data.gene_prioritisation.results_dir.name}" | ||
| RankComparisonGenerator(prioritisation_data.gene_prioritisation.ranks).generate_output( | ||
| f"{prioritisation_data.gene_prioritisation.results_dir.name}", "-gene_rank_comparison.tsv" |
There was a problem hiding this comment.
Technically this filename seems to violate SRP (because it exists more than once in the code).
There was a problem hiding this comment.
The construction of the full filename? Or the "-gene_rank_comparison.tsv" part?
There was a problem hiding this comment.
The string "-gene_rank_comparison.tsv" appears twice in that file.
There was a problem hiding this comment.
Ah yes! I plan on addressing this in #193. I have the code roughly written out with how I plan it to be structured and it avoids this (used a constant for _rank_comparison.tsv)
| def generate_gene_output(self, prefix: str) -> None: | ||
| """Generate the output for gene prioritisation ranks.""" | ||
| self._generate_dataframe().to_csv(prefix + "-gene_rank_comparison.tsv", sep="\t") | ||
| def generate_output(self, prefix: str, suffix: str): |
There was a problem hiding this comment.
You are a tiny bit inconsistent in whether a function that does not return anything should end in -> None or not
There was a problem hiding this comment.
Yep, you're right! I'll fix this
yaseminbridges
deleted the
186-generalise-rankcomparisongenerator-class
branch
Generalised methods in the
RankComparisonGeneratorclass