Hotfix plotting issues reversed date axis

Bugfix in plotHistogram unittest message Bugfix in plotPeakWidth due to ggplot2 change in behaviour Update github action
phenomecentre · Dec 12, 2022 · 556094d · 556094d
2 parents 8d1277f + 8929d1b
commit 556094d
Show file tree

Hide file tree

Showing 11 changed files with 140 additions and 137 deletions.
diff --git a/.github/workflows/check-bioc-devel.yml b/.github/workflows/check-bioc-devel.yml
@@ -53,9 +53,9 @@ jobs:
       fail-fast: false
       matrix:
         config:
-          - { os: ubuntu-latest, r: 'devel', bioc: '3.16', cont: "bioconductor/bioconductor_docker:devel", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
-          - { os: macOS-latest, r: 'devel', bioc: '3.16'}
-          - { os: windows-latest, r: 'devel', bioc: '3.16'}
+          - { os: ubuntu-latest, r: 'devel', bioc: '3.17', cont: "bioconductor/bioconductor_docker:devel", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
+          - { os: macOS-latest, r: 'devel', bioc: '3.17'}
+          - { os: windows-latest, r: 'devel', bioc: '3.17'}
     env:
       R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
       RSPM: ${{ matrix.config.rspm }}
@@ -75,22 +75,22 @@ jobs:
           echo ".libPaths('/__w/_temp/Library')" > ~/.Rprofile
 
       ## Most of these steps are the same as the ones in
-      ## https://github.com/r-lib/actions/blob/master/examples/check-standard.yaml
+      ## https://github.com/r-lib/actions/blob/v2/examples/check-standard.yaml
       ## If they update their steps, we will also need to update ours.
       - name: Checkout Repository
         uses: actions/checkout@v2
 
       ## R is already included in the Bioconductor docker images
       - name: Setup R from r-lib
         if: runner.os != 'Linux'
-        uses: r-lib/actions/setup-r@master
+        uses: r-lib/actions/setup-r@v2
         with:
           r-version: ${{ matrix.config.r }}
 
       ## pandoc is already included in the Bioconductor docker images
       - name: Setup pandoc from r-lib
         if: runner.os != 'Linux'
-        uses: r-lib/actions/setup-pandoc@master
+        uses: r-lib/actions/setup-pandoc@v2
 
       - name: Query dependencies
         run: |
@@ -269,7 +269,7 @@ jobs:
 
       - name: Upload check results
         if: failure()
-        uses: actions/upload-artifact@master
+        uses: actions/upload-artifact@v2
         with:
           name: ${{ runner.os }}-biocversion-devel-r-devel-results
           path: check
diff --git a/.github/workflows/check-bioc.yml b/.github/workflows/check-bioc.yml
@@ -53,9 +53,9 @@ jobs:
       fail-fast: false
       matrix:
         config:
-          - { os: ubuntu-latest, r: '4.2', bioc: '3.15', cont: "bioconductor/bioconductor_docker:RELEASE_3_15", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
-          - { os: macOS-latest, r: '4.2', bioc: '3.15'}
-          - { os: windows-latest, r: '4.2', bioc: '3.15'}
+          - { os: ubuntu-latest, r: '4.2', bioc: '3.16', cont: "bioconductor/bioconductor_docker:RELEASE_3_16", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
+          - { os: macOS-latest, r: '4.2', bioc: '3.16'}
+          - { os: windows-latest, r: '4.2', bioc: '3.16'}
     env:
       R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
       RSPM: ${{ matrix.config.rspm }}
@@ -75,22 +75,22 @@ jobs:
           echo ".libPaths('/__w/_temp/Library')" > ~/.Rprofile
 
       ## Most of these steps are the same as the ones in
-      ## https://github.com/r-lib/actions/blob/master/examples/check-standard.yaml
+      ## https://github.com/r-lib/actions/blob/v2/examples/check-standard.yaml
       ## If they update their steps, we will also need to update ours.
       - name: Checkout Repository
         uses: actions/checkout@v2
 
       ## R is already included in the Bioconductor docker images
       - name: Setup R from r-lib
         if: runner.os != 'Linux'
-        uses: r-lib/actions/setup-r@master
+        uses: r-lib/actions/setup-r@v2
         with:
           r-version: ${{ matrix.config.r }}
 
       ## pandoc is already included in the Bioconductor docker images
       - name: Setup pandoc from r-lib
         if: runner.os != 'Linux'
-        uses: r-lib/actions/setup-pandoc@master
+        uses: r-lib/actions/setup-pandoc@v2
 
       - name: Query dependencies
         run: |
@@ -103,16 +103,16 @@ jobs:
         uses: actions/cache@v2
         with:
           path: ${{ env.R_LIBS_USER }}
-          key: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_15-r-4.2-${{ hashFiles('.github/depends.Rds') }}
-          restore-keys: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_15-r-4.2-
+          key: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_16-r-4.2-${{ hashFiles('.github/depends.Rds') }}
+          restore-keys: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_16-r-4.2-
 
       - name: Cache R packages on Linux
         if: "!contains(github.event.head_commit.message, '/nocache') && runner.os == 'Linux' "
         uses: actions/cache@v2
         with:
           path: /home/runner/work/_temp/Library
-          key: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_15-r-4.2-${{ hashFiles('.github/depends.Rds') }}
-          restore-keys: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_15-r-4.2-
+          key: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_16-r-4.2-${{ hashFiles('.github/depends.Rds') }}
+          restore-keys: ${{ env.cache-version }}-${{ runner.os }}-biocversion-RELEASE_3_16-r-4.2-
 
       - name: Install Linux system dependencies
         if: runner.os == 'Linux'
@@ -267,7 +267,7 @@ jobs:
 
       - name: Upload check results
         if: failure()
-        uses: actions/upload-artifact@master
+        uses: actions/upload-artifact@v2
         with:
-          name: ${{ runner.os }}-biocversion-RELEASE_3_15-r-4.2-results
+          name: ${{ runner.os }}-biocversion-RELEASE_3_16-r-4.2-results
           path: check
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,7 +1,7 @@
 Package: peakPantheR
 Title: Peak Picking and Annotation of High Resolution Experiments
-Version: 1.13.0
-Date: 2022-08-05
+Version: 1.13.1
+Date: 2022-11-06
 Authors@R: c(person("Arnaud", "Wolfer", email = "[email protected]", role = c("aut", "cre"), comment = c(ORCID = "0000-0001-5856-3218")),
 	person("Goncalo", "Correia", email = "[email protected]", role = "aut", comment = c(ORCID = "0000-0001-8271-9294")),
 	person("Jake", "Pearce", email = "[email protected]", role = "ctb"),
@@ -28,7 +28,8 @@ Imports:
 	shinycssloaders (>= 1.0.0),
 	DT (>= 0.15),
 	pracma (>= 2.2.3),
-	utils
+	utils,
+	lubridate
 biocViews: MassSpectrometry, Metabolomics, PeakDetection
 License: GPL-3
 BugReports: https://github.com/phenomecentre/peakPantheR/issues/new

diff --git a/R/peakPantheR_plotPeakwidth.R b/R/peakPantheR_plotPeakwidth.R
@@ -60,41 +60,44 @@
 peakPantheR_plotPeakwidth <- function(apexValue, widthMin=NULL, widthMax = NULL,
     acquTime=NULL, varName="variable", sampleColour = NULL, rotateAxis = FALSE,
     verbose = TRUE) {
-    
-    # Check input
+
+    ## Check input
     resInp <- plotPeakwidth_checkInput(apexValue, widthMin, widthMax, acquTime,
                                         sampleColour, verbose)
     nbSpl <- resInp$nbSpl; useRunOrder <- resInp$useRunOrder
     useWidth <- resInp$useWidth; colourSpl <- resInp$colourSpl
     sampleIDColour <- resInp$sampleIDColour
 
-    # Init plot draw default x/y with y the variable, rotate later if required
+    ## Init plot draw default x/y will fill depending on rotation
     p <- ggplot2::ggplot(NULL, ggplot2::aes(x), environment = environment()) +
-        ggplot2::theme_bw() +
-        ggplot2::ylab(varName)
-
+        ggplot2::theme_bw()
     # set fill and colour scale, with one color per sample ID
     p <- p + ggplot2::scale_colour_manual(values = colourSpl, guide = "none")
     p <- p + ggplot2::scale_fill_manual(values = colourSpl, guide = "none")
 
-    # tmp x axis
+    ## tmp main axis 
     if (useRunOrder) {
         x_axis <- acquTime
     } else {
-        if (rotateAxis) {
-            x_axis <- seq(nbSpl - 1, 0)  #first spectra on top
-        } else {
-            x_axis <- seq(0, nbSpl - 1)  #first spectra on left
-        }
+        x_axis <- seq(0, nbSpl - 1)  #first spectra on left
+    }
+
+    ## Plot peakwidth, apex values and apex points (flip x/y if rotateAxis)
+    # must rotateAxis inside the function otherwise the checks for NA wouldn't 
+    # work with flipped inputs
+    p <- plotPeakwidth_plotWidthApex(p, useWidth, rotateAxis, x_axis,
+                    widthMin, widthMax, sampleIDColour, apexValue, verbose)
+
+    # some expected messages
+    if (verbose) {
+        if (useRunOrder) { message("Values plotted by run order")
+        } else {           message("Values plotted by input order") }
     }
-
-    # Plot peakwidth, apex values and apex points
-    p <- plotPeakwidth_plotWidthApex(p, useWidth, useRunOrder, x_axis, widthMin,
-                        widthMax, sampleIDColour, apexValue, acquTime, verbose)
 
-    # Rotate axis
-    p <- plotPeakwidth_rotateAxis(p, rotateAxis, useRunOrder, verbose)
-
+    ## final ordering and axis labelling
+    p <- plotPeakwidth_rotateAxis_setLabels(p, rotateAxis, useRunOrder, 
+                                            x_axis, acquTime, varName, verbose)
+
     return(p)
 }
 
@@ -153,98 +156,94 @@ plotPeakwidth_checkInput <- function(apexValue, widthMin, widthMax, acquTime,
     return(list(nbSpl=nbSpl, useRunOrder=useRunOrder, useWidth=useWidth,
                 colourSpl=colourSpl, sampleIDColour=sampleIDColour))
 }
-# Plot peakwidth, apex values and apex points
-plotPeakwidth_plotWidthApex <- function(p, useWidth, useRunOrder, x_axis,
-            widthMin, widthMax, sampleIDColour, apexValue, acquTime, verbose) {
-    # peak width (must go first to be the bottom most layer)
+# Plot peakwidth, apex values and apex points (flip x/y if rotateAxis)
+plotPeakwidth_plotWidthApex <- function(p, useWidth, rotateAxis, 
+            x_axis, widthMin, widthMax, sampleIDColour, apexValue, verbose) {
+
+    ## Apex value
+    # add apex point (add the color ID to each point, and an ID pointing to the 
+    # black stroke)
+    tmp_pt <- data.frame(x = x_axis, y = apexValue, colr = sampleIDColour,
+                        stringsAsFactors = FALSE)
+    tmp_pt <- tmp_pt[!is.na(tmp_pt$y), ]
+
+    # with peakwidth and black stroke
     if (useWidth) {
-        # value peakwidth (add color ID to each point)
+        ## Prepare peakWidth (must go first to be the bottom most layer)
         tmp_pwidth <- data.frame(x = c(x_axis, x_axis),
                                 y = c(widthMin, widthMax),
                                 colr = c(sampleIDColour, sampleIDColour),
                                 stringsAsFactors = FALSE)
-        tmp_pwidth <- tmp_pwidth[!is.na(tmp_pwidth$y), ]
-        p <- p + ggplot2::geom_line(data = tmp_pwidth,
-                    ggplot2::aes(x = x, y = y, group = x, colour=colr), size=1)
+        tmp_pwidth <- tmp_pwidth[!is.na(tmp_pwidth$y),]#remove val without pkwdt
+        if (rotateAxis) { #flip x and y
+            p <- p + ggplot2::geom_line(data=tmp_pwidth,
+                ggplot2::aes(x=y, y=x, group=x, colour=colr), linewidth=1)
+            p <- p + ggplot2::geom_point(data = tmp_pt, 
+                ggplot2::aes(x=y, y=x, fill=colr), 
+                colour="black", shape=21, stroke=1, size=2)
+        } else {
+            p <- p + ggplot2::geom_line(data = tmp_pwidth, 
+                ggplot2::aes(x=x, y=y, group=x, colour=colr), linewidth=1)
+            p <- p + ggplot2::geom_point(data = tmp_pt, 
+                ggplot2::aes(x=x, y=y, fill=colr), 
+                colour="black", shape=21, stroke=1, size=2)
+        }
         if (verbose) { message("Peakwidth values plotted") }
-    }
-
 
-    # apex value
-    if (useRunOrder) {
-        p <- p + ggplot2::xlab("Acquisition Time")
-                #labels are flipped, themes are not
-        tmp_pt <- data.frame(x = acquTime, y = apexValue, colr = sampleIDColour,
-            stringsAsFactors = FALSE)
-        if (verbose) { message("Values plotted by run order") }
+    # without peakwidth and black stroke
     } else {
-        # labels are flipped, themes are not
-        # add apex point (add the color ID to each point, and an ID pointing to
-        # the black stroke)
-        tmp_pt <- data.frame(x = x_axis, y = apexValue, colr = sampleIDColour,
-                            stringsAsFactors = FALSE)
-        if (verbose) { message("Values plotted by input order") }
-    }
-    # add apex points
-    tmp_pt <- tmp_pt[!is.na(tmp_pt$y), ]
-    # with black stroke
-    if (useWidth) {
-        p <- p + ggplot2::geom_point(data = tmp_pt,
-                    ggplot2::aes(x = x, y = y, fill = colr), colour = "black",
-                                shape = 21, stroke = 1, size = 2)
-        # without black stroke
-    } else {
-        p <- p + ggplot2::geom_point(data = tmp_pt,
-                    ggplot2::aes(x = x, y = y, colour = colr))
+        if (rotateAxis) { #flip x and y
+            p <- p + ggplot2::geom_point(data=tmp_pt,
+                ggplot2::aes(x=y, y=x, colour=colr))
+        } else {
+            p <- p + ggplot2::geom_point(data = tmp_pt, 
+                ggplot2::aes(x=x, y=y, colour=colr))
+        }
     }
-
     return(p)
 }
-# Rotate axis
-plotPeakwidth_rotateAxis <- function(p, rotateAxis, useRunOrder, verbose) {
-    # rotate axis (labels are carried with the flip, but themes are applied
-    # afterwards)
-    if (rotateAxis) {
-        p <- p + ggplot2::coord_flip()
-
-        if (useRunOrder) { # Run order
-            # if date axis vertical, put dates in order from top to bottom
-            # function to reverse date axis
-            c_trans <- function(a, b, breaks = b$breaks, format = b$format) {
-                a <- scales::as.trans(a)
-                b <- scales::as.trans(b)
-                name <- paste(a$name, b$name, sep = "-")
-                trans <- function(x) a$trans(b$trans(x))
-                inv <- function(x) b$inverse(a$inverse(x))
-                scales::trans_new(name = name, transform = trans, inverse = inv,
-                                breaks = breaks, format = format)
-            }
-            rev_date <- c_trans("reverse", "time")
-
-            p <- p + ggplot2::theme(
-                axis.text.y = ggplot2::element_text(angle = 45, hjust = 1)) +
-                ggplot2::scale_x_continuous(trans = rev_date,
-                                            expand = c(0.007, 0.007))
-        } else { # no run order
-            p <- p + ggplot2::theme(axis.title.y = ggplot2::element_blank(),
-                                    axis.text.y = ggplot2::element_blank(),
-                                    axis.ticks.x = ggplot2::element_blank()) +
-                ggplot2::scale_x_continuous(breaks = NULL,
-                                            expand = c(0.007, 0.007))
-        }
-        if (verbose) { message("x and y axis rotated") }
-
-    } else { # no rotation
-        if (useRunOrder) {
-            p <- p + ggplot2::theme(
-                axis.text.x = ggplot2::element_text(angle = 45, hjust = 1)) +
+# Set labels and rotate axis if necessary
+plotPeakwidth_rotateAxis_setLabels <- function(p, rotateAxis, useRunOrder, 
+                                        x_axis, acquTime, varName, verbose) {
+    coord_y_datetime <- function(xlim = NULL, ylim = NULL, expand = TRUE) {
+        if (!is.null(ylim)) { ylim <- lubridate::as_datetime(ylim) }
+        ggplot2::coord_cartesian(xlim = xlim, ylim = ylim, expand = expand)
+    }
+    ## Ordering of values and axis labelling
+    if (useRunOrder) {
+        if (rotateAxis) { # date axis rotated (+ flipped)
+            p <- p + coord_y_datetime(ylim = c(max(acquTime), min(acquTime))) +
+                ggplot2::xlab(varName) + 
+                ggplot2::ylab("Acquisition Time") +
+                ggplot2::theme(
+                        axis.text.y=ggplot2::element_text(angle=45, hjust=1)) +
+                ggplot2::scale_y_datetime(expand = c(0.007, 0.007))
+            if (verbose) { message("x and y axis rotated") }
+        } else {          # date axis not rotated
+            p <- p + ggplot2::xlab("Acquisition Time") +
+                ggplot2::ylab(varName) + 
+                ggplot2::theme(
+                        axis.text.x=ggplot2::element_text(angle=45, hjust=1)) +
                 ggplot2::scale_x_datetime(expand = c(0.007, 0.007))
-        } else {
-            p <- p + ggplot2::theme(axis.title.x = ggplot2::element_blank(),
-                                    axis.text.x = ggplot2::element_blank(),
-                                    axis.ticks.x = ggplot2::element_blank()) +
-                ggplot2::scale_x_continuous(breaks = NULL,
-                                            expand = c(0.007, 0.007))
+        }
+    } else {
+        if (rotateAxis) { # input order axis rotated (+ flipped)
+            suppressMessages(
+            p <- p +  #needs to go first
+                ggplot2::scale_y_continuous(breaks=NULL, expand=c(0.007,0.007))+
+                ggplot2::ylim(max(x_axis), min(x_axis)) +
+                ggplot2::xlab(varName) + 
+                ggplot2::theme(axis.title.y = ggplot2::element_blank(),
+                                axis.text.y = ggplot2::element_blank(),
+                                axis.ticks.y = ggplot2::element_blank())
+            )
+            if (verbose) { message("x and y axis rotated") }
+        } else {          # input order axis not rotated
+            p <- p + ggplot2::ylab(varName) + 
+                ggplot2::theme(axis.title.x = ggplot2::element_blank(),
+                                axis.text.x = ggplot2::element_blank(),
+                                axis.ticks.x = ggplot2::element_blank()) +
+                ggplot2::scale_x_continuous(breaks=NULL, expand=c(0.007,0.007))
         }
     }
     return(p)

diff --git a/R/plotEICDetectedPeakwidth.R b/R/plotEICDetectedPeakwidth.R
@@ -57,7 +57,7 @@ plotEICDetectedPeakwidth <- function(ROIDataPointSampleList, cpdID, cpdName, rt,
     maxX <- max(ggplot2::layer_scales(p_spec)$x$range$range[2],
         ggplot2::layer_scales(p_peakwidth)$y$range$range[2])
     p_spec <- p_spec + ggplot2::xlim(minX, maxX)
-    p_peakwidth <- p_peakwidth + ggplot2::ylim(minX, maxX)
+    suppressMessages(p_peakwidth <- p_peakwidth + ggplot2::ylim(minX, maxX))
     # convert to gtables
     p_spec <- ggplot2::ggplot_gtable(ggplot2::ggplot_build(p_spec))
     p_peakwidth <- ggplot2::ggplot_gtable(ggplot2::ggplot_build(p_peakwidth))

diff --git a/R/plotHistogram.R b/R/plotHistogram.R
@@ -19,7 +19,7 @@ plotHistogram <- function(var, varName = "Variable", density = TRUE, ...) {
     # with density
     if (density) {
         p_hist <- p_hist +
-                    ggplot2::geom_histogram(ggplot2::aes(y = ..density..),
+                    ggplot2::geom_histogram(ggplot2::aes(y=ggplot2::after_stat(density)),
                         colour = "black", fill = "white", ...) +
                     ggplot2::geom_density(alpha = 0.1, fill = "blue")
         # without density