Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for Issue #24 #198

Closed
wants to merge 3 commits into from
Closed

Fix for Issue #24 #198

wants to merge 3 commits into from

Conversation

Max-Bladen
Copy link
Collaborator

This issue ended up being more multifaceted than originally thought.

--- Prior issue: ---
While exploring the bug, a separate (but related) issue was noticed. On standard (s)plsda objects (ie. non-MINT), only some of the background.predict() regions (or polygons) would display. Only thing that resolved this issue was manually adjusting the xlim and ylim parameters (increasing their bounds). This occured only for style = ggplot2. The ggplot2 package entirely removes polygons from the plot if a certain percentage of said polygon is outside the range of the axes (ie, if plot too zoomed in, polygon won't show). Previously internal_graphicModule() had used the bounds of the points as the default xlim and ylim, resulting in this cutoff.

Hence, to resolve this, if a background is supplied (and xlim/ylim are not supplied) to plotIndiv(), then the default value of the limit parameters is set to the average of the minimum/maximum values of the points and the polygons. This allows the plot to not be too 'zoomed out' as well as the polygons to be shown.

--- Immediate Issue: ---
The error raised when trying to use dist = "max.dist" in background.predict() on a mint.splsda object was caused by the absence of the center and scale attributes on the mint.splsda$ind.mat object. It had these for each study individually, but global values are needed for background.predict(). Using the follow externally would result in incorrect values for these attributes:
mint.res.object$ind.mat <- scale(mint.res.object$ind.mat)

Hence, mean_centering_per_study() was adjusted such that if the center and scale parameters were TRUE, irrespective of whether it is a MINT object or not, the global center and scale attributes will be appended to the ind.mat object.

--- Downstream issue: ---
The above fix resolved the error noted in Issue #24, however upon using the plotIndiv() function with a supplied background (on the mint.splsda object), no background would be shown. This was due to the absence of any code which handles background polygons within the if (style == "ggplot2-MINT") block (starts on line 364 of internal_graphicModule()).

Hence, the code from the if (style == "ggplot2") which handles backgrounds was copied over into the MINT block. On top of this, plotIndiv.mint() was slightly adjusted so the names(background.object) are converted into colours. This is how it is handled in the standard sPLSDA case so this process was homogenised for consistency's sake.

@Max-Bladen Max-Bladen added the bug-fix For PR's that address an Issue with `bug` label label Mar 28, 2022
@Max-Bladen Max-Bladen self-assigned this Mar 28, 2022
@Max-Bladen Max-Bladen linked an issue Mar 28, 2022 that may be closed by this pull request
`plotIndiv()` documentation needed updated to ensure all relevant functions have the `background` parameter
Coverage decreased after latest push. Adding more tests should fix this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix For PR's that address an Issue with `bug` label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add background.predict support for mint.splsda
1 participant