pritykinlab / guidescanpy

1 stars 0 forks source link

Refactoring #57

Closed vineetbansal closed 1 year ago

vineetbansal commented 1 year ago

Sorry I didn't notice this before, but I don't see why this block needs to be in a while loop, and why make up a new "unknown" value when None will do the job (it's okay to return either a string or a None in python and handle the two cases in the caller):

https://github.com/pritykinlab/guidescanpy/blob/17fd80665a6f1b86e26b9344fd0708e0251f1771/src/guidescanpy/commands/add_tag.py#L63

In fact I don't think there's a need of a get_format function at all since the logic is so straightforward.

1rzhu commented 1 year ago

The logic of the while loop is that I wanted to determine the output format by the file extension, but since we may have files with multiple extensions, like .bam.sorted, I used the loop to go through all extensions.

I agree that "unknown" should be replaced by None. And I think if we want to get rid of this get_format function, we can just detect the last extension, if it's .sam, then output a sam file, otherwise we just give a bam.

vineetbansal commented 1 year ago

Thanks for the clarification! In that case let's just use .endswith(".bam") or .endswith(".bam.sorted"). I think that makes it easier to read.