search

microbiome / miaViz

Microbiome Analysis Plotting and Visualization
https://microbiome.github.io/miaViz
Artistic License 2.0
10 stars 12 forks source link

Update params in user facing fxns #134

Closed Daenarys8 closed 4 months ago

Daenarys8 commented 4 months ago

This commit updates parameters of user functions.

TuomasBorman commented 4 months ago

plotPrevalence has parameter called label. plotGraph has the same parameter called show.label. I think they are doing the same thing, but "label" is little bit more unclear. -> can you check that they do the same thing and rename them to show.label?

Daenarys8 commented 4 months ago

plotPrevalence has parameter called label. plotGraph has the same parameter called show.label. I think they are doing the same thing, but "label" is little bit more unclear. -> can you check that they do the same thing and rename them to show.label?

Hmm, label focuses on rownames while show.label handles logical vectors for selective labeling and matches character vectors to node data columns i.e label is used to specify which labels from the rownames of the data should be selected and show.label controls whether and which tip labels are plotted.

Should I go ahead and rename to show.lable?

TuomasBorman commented 4 months ago

plotPrevalence has parameter called label. plotGraph has the same parameter called show.label. I think they are doing the same thing, but "label" is little bit more unclear. -> can you check that they do the same thing and rename them to show.label?

Hmm, label focuses on rownames while show.label handles logical vectors for selective labeling and matches character vectors to node data columns i.e label is used to specify which labels from the rownames of the data should be selected and show.label controls whether and which tip labels are plotted.

Should I go ahead and rename to show.lable?

I think the idea is still the same; select labels that are going to be shown in the plot? Right? I think then the names should be equal (at least for me it was not intuitive what label do. For show.label it is much more clear what it does; controls labels)

Daenarys8 commented 4 months ago

plotPrevalence has parameter called label. plotGraph has the same parameter called show.label. I think they are doing the same thing, but "label" is little bit more unclear. -> can you check that they do the same thing and rename them to show.label?

Hmm, label focuses on rownames while show.label handles logical vectors for selective labeling and matches character vectors to node data columns i.e label is used to specify which labels from the rownames of the data should be selected and show.label controls whether and which tip labels are plotted. Should I go ahead and rename to show.lable?

I think the idea is still the same; select labels that are going to be shown in the plot? Right? I think then the names should be equal (at least for me it was not intuitive what label do. For show.label it is much more clear what it does; controls labels)

I agree. show.label is more intuitive. Will add the change.

TuomasBorman commented 4 months ago

I checked the unit tests. There are many tests that test internal functions, and parameter names of internal functions are not changed if they cannot be used by user. Unit tests are updated and should work after deprecated parameter names are removed in the future.