microbiome / mia

Microbiome analysis
https://microbiome.github.io/mia/
Artistic License 2.0
46 stars 25 forks source link

Getter and setters #484

Closed Daenarys8 closed 2 weeks ago

Daenarys8 commented 5 months ago

Ping #480

TuomasBorman commented 5 months ago

So to summarize: it is better to modify only those lines where it is relevant. You can do another PR for removing additional spaces if we see that necessary

TuomasBorman commented 5 months ago

Hello!

Sorry that it took so long. I made changes to mia:getters_and_setters branch. Please see those. They are related to agglomerateByRank function. Now, MARGIN is supported meaning that we can utilize this function also to columns (altexp was already working...).

devtools::load_all()

data("GlobalPatterns")

tse <- GlobalPatterns

# Agglomerate to all ranks
altExps(tse) <- splitByRanks(tse)

# Agglomerate altExp
altExp(tse, "test") <- tse[1:10, ]
tse_altExp <- agglomerateByRank(tse, rank = "Kingdom", altexp = "test")
tse_altExp

# Create ranks for columns
colData(tse)[["Kingdom"]] <- sample(c("king1", "king2", "king3"), size = ncol(tse), replace = TRUE)
colData(tse)[["Class"]] <- sample(c("class1"), size = ncol(tse), replace = TRUE)
colData(tse)[["Species"]] <- sample(c("species1", "species2", "species3", "species4"), size = ncol(tse), replace = TRUE)
colData(tse)

# Agglomerate columns
tse_col <- agglomerateByRank(tse, rank = "Kingdom", MARGIN = 2)
tse_col

tse_col <- agglomerateByRank(tse, rank = "Class", MARGIN = 2)
colData(tse_col)

Can you give feedback on that? Is there anything that could break? Is it too complex solution / is it easy to follow what happens? If everything is good, I think we can proceed again.

antagomir commented 4 months ago

@Daenarys8 any chance to resolve the conflicts and finalize this one?

Daenarys8 commented 4 months ago

ACK. Will push an update asap.

antagomir commented 3 weeks ago

This is old PR, still open. What's the status of this one @Daenarys8 ?

TuomasBorman commented 3 weeks ago

This might be something that we should check in the future, not now. This would be good addition, but requires lots of work and I am not sure if it is worth to do at least now when we have other tasks

antagomir commented 3 weeks ago

Ok. I was just worried about having too many active open PRs.

TuomasBorman commented 2 weeks ago

Closing for now