Bug: Slope Selector Plots Incorrect

[js3110]

Issue

Closes #165

Description

The slope selector plots do not always match the NCA results, showing the wrong line of best fit or points included in lambda z. This is best noticed when comparing the output of one analyte vs multiple analytes for the same subject profile.
For some multiple analyte studies, the slope plots crash with error message: Warning in max(lambda_z_ix_rows$AVAL) : no non-missing arguments to max; returning -Inf Warning: Error in if: missing value where TRUE/FALSE needed 87: lambda_slope_plot [/aNCA/R/lambda_slope_plot.R#134]

Looks like lamda.z.ix is being calculated incorrectly.
In addition, if user uses the table and selects 1:8 as manual slope, the selection includes 2,3,4,5,6,7 but not 1 and 8. Would be better if it was inclusive of the range the user inputs.

Definition of Done

• Slope plots should correspond exactly to the NCA results, and be the same no matter how many analytes / pcspec have been selected.
• Exclusions and inclusions should work fine, for both manual table adding or adding via interactive plots
• Inclusions should include the IX values the user inputs
• Should work for multiple analytes without crashing
• Include entire range of points used to calculate lambda z

Contributor checklist

• Code passes lintr checks
• Code passes all unit tests
• New logic covered by unit tests
• New logic is documented

Notes to reviewer

As well as fixing the bug issues, I have also refactored the code to avoid hardcoding the variables and instead created slopes_groups() reactive which takes the column names from mydata()$conc$columns$groups

[Gero1999]

I will provide comments to the code as you requested @js3110!

[js3110]

Hi @m-kolomanski I have pushed some changes based on your comments. Regarding some of the functions, I am not sure if I have put them in the right place or if they need further documentation added, so please let me know!

[Gero1999]

Bugs detected

• Plots per page input failed when changing it. I tested with DummyRO_with_metab, using multiple analyte. The error says:

TRACE [2025-02-04 11:39:30] slope_selector: Updating displayed plots
Warning: Error in if: argument is of length zero

• With all default settings run, doing a selection in the first plot of Point4 > Point7 did add a row to the slopes table but did not change the plot

UI suggestions

• It is super good that is good the plots have a standard size. But I feel they are way to big, which makes difficult comparison. If we are keen on keeping this size I would suggest increasing then the axis size

[Gero1999]

question: do we really need this dependency? jsonlite::fromJSON / jsonlite::toJSON
Maybe is easier to just make identifiers a regular dataframe with dplyr or base?

[js3110]

I don't mind, I did this because I had an issue with "_" in the value names causing the identifiers to be split wrong, but I'm sure it could be manually coded. What do you think is best @m-kolomanski @Gotfrid ?

[js3110]

Bugs detected

• Plots per page input failed when changing it. I tested with DummyRO_with_metab, using multiple analyte. The error says:

As discussed, we have a new issue #192 to solve this, as its not related to this issue.

• With all default settings run, doing a selection in the first plot of Point4 > Point7 did add a row to the slopes table but did not change the plot

This is covered in issue #121 so I will leave it for there

UI suggestions

• It is super good that is good the plots have a standard size. But I feel they are way to big, which makes difficult comparison. If we are keen on keeping this size I would suggest increasing then the axis size

I agree, the problem is when I make them smaller, they are too small on the small screen. At the moment if you have more than 2 plots per page they just appear underneath in the same size, and you have to scroll down to view, which makes them too big on the big screen. It would be good if there was a way to set the container size and then even if you add more plots per page they stay within the container. Do you know how to do this @m-kolomanski ? My knowledge of css and styling is limited 😨

[js3110]

@Gero1999 @m-kolomanski @Gotfrid I have had a go at changing the styling for the plots. On a big screen, it works fairly nicely and as intended, however on the small screen for some reason it disregards everything that is coded in css and just does whatever it wants. I can't seem to solve that 😅 .
The other issue I have noticed is that the "previous_page"and "next_page" buttons are not disabled when they are supposed to be, which means the user can click when they shouldn't and cause a crash. I was also unable to solve this...
So I would appreciate some help ! Thanks

[Gero1999]

Bugs detected

• Plots per page input failed when changing it. I tested with DummyRO_with_metab, using multiple analyte. The error says:

As discussed, we have a new issue #192 to solve this, as its not related to this issue.

As we talked I rebased the branch to solve the multiple analytes metabolites case, so we can use DummyRO_with_metab.csv for the testing