thackl / gggenomes

A grammar of graphics for comparative genomics
https://thackl.github.io/gggenomes/
Other
605 stars 65 forks source link

Unused parameter in drop_feat_layout #172

Closed iimog closed 8 months ago

iimog commented 10 months ago

While documenting the parameters, I think I found a bug. But before I fix it, I wanted to be sure that it is actually a bug and not an obscure feature :smile:.

The function drop_feat_layout has a positional seqs parameter, that is never used: https://github.com/thackl/gggenomes/blob/f29495a25cc4168946ab7897852e920f926e208a/R/feats.R#L93-L97

If I understand correctly, this leads to the keep parameter of layout_feats being ignored, as it calls drop_feat_layout like this: https://github.com/thackl/gggenomes/blob/f29495a25cc4168946ab7897852e920f926e208a/R/feats.R#L78

So the keep variable is interpreted as seqs and ignored, while the keep parameter still uses the default value.

@thackl if you agree, that this is a bug, I will just delete the seqs parameter from the function definition.

The same applies to the drop_link_layout function. https://github.com/thackl/gggenomes/blob/f29495a25cc4168946ab7897852e920f926e208a/R/links.R#L149-L153

thackl commented 10 months ago

Yap. I think you are quite right. Not sure though, why this doesn't seem to break anything as it is...

thackl commented 10 months ago

Ah, because whenever I use it, I want to keep "strand". And that is the default behavior even if the positional seqs argument eats up the unnamed keep parameter with "strand" as value 🙃 ..

So yes, feel free to drop the seqs argument!