search

stemangiola / tidySingleCellExperiment

Brings SingleCellExperiment objects to the tidyverse
https://stemangiola.github.io/tidySingleCellExperiment/index.html
35 stars 10 forks source link

Allow y as S4 DataFrame in *_join() #98

Closed lambdamoses closed 11 months ago

lambdamoses commented 11 months ago

Address #80

stemangiola commented 11 months ago

Somehow your comment about refactoring disappeared, I must have clicked on something weird.

Regarding your refectory, I cannot figure out whether your factoring proposal would only decrease complexity or also increase complexity somehow. If it's not a lot of work for you to re-factor the code, can I please ask you to do so in a separate branch so we can evaluate whether to keep or reject the proposal?

On the other hand, if it's a lot of work, there might be other priorities beyond this.

lambdamoses commented 11 months ago

No, it isn't a lot for work. It will decrease the complexity, since it's really one function behind all 4 join flavors. I'll do it in a branch in my fork of your repo. However, it might make debugging a bit more complicated.

lambdamoses commented 11 months ago

Somehow your comment about refactoring disappeared, I must have clicked on something weird.

The comment is in the issue, not this pull request.

lambdamoses commented 11 months ago

See the refactoring here: https://github.com/lambdamoses/tidySingleCellExperiment/blob/bcd3f5529948e9cff2f317bbf001558db9986bae/R/dplyr_methods.R#L329

stemangiola commented 11 months ago

Amazing, if you feel you can tackle the same for tidySE it would be amazing so we keep the ecosystem consistent.