wwood / galah

More scalable dereplication for metagenome assembled genomes
GNU General Public License v3.0
48 stars 11 forks source link

Refactor clusterer choice #34

Closed wwood closed 1 year ago

wwood commented 1 year ago

Thoughts about the last commit @AroneyS ? I actually didn't think hard about this very much - copilot did most of the coding, but seems pretty straightforward.

The tests don't pass, I guess because of the 100 issue you mentioned. If you like this can you take it to completion please?

Thanks, ben

AroneyS commented 1 year ago

Well that is much easier than I was thinking. I couldn't work out how to use the internal clusterer within the enum. But accessing the methods directly makes sense. I'll see if I can fix the tests.

AroneyS commented 1 year ago

Is there a reason that parse_percentage returns a decimal? https://github.com/wwood/galah/blob/31a6edd14e3417ba4cb69744fb8402f1e414a23a/src/cluster_argument_parsing.rs#L1015 Seems odd to do that and then multiply by 100 when we need it...

wwood commented 1 year ago

I guess it's just that the decimal is a bit human. Also worried a change might break coverm etc

-------------- Ben Woodcroft Group leader, Centre for Microbiome Research, QUT


From: Samuel Aroney @.> Sent: Wednesday, August 30, 2023 8:55:11 AM To: wwood/galah @.> Cc: Ben J Woodcroft @.>; Author @.> Subject: Re: [wwood/galah] Refactor clusterer choice (PR #34)

Is there a reason that parse_percentage returns a decimal? https://github.com/wwood/galah/blob/31a6edd14e3417ba4cb69744fb8402f1e414a23a/src/cluster_argument_parsing.rs#L1015 Seems odd to do that and then multiply by 100 when we need it...

― Reply to this email directly, view it on GitHubhttps://github.com/wwood/galah/pull/34#issuecomment-1698248299, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAADX5DZAMMTP7GF3AT3DI3XXZXM7ANCNFSM6AAAAAA4CQKBHI. You are receiving this because you authored the thread.Message ID: @.***>

wwood commented 1 year ago

superseded by https://github.com/wwood/galah/pull/33