Fix group_level when printing a character of a number

[barrettk]

This PR removes a section that prevented numeric group_levels from being used.

Previously, these values would be overwritten with empty quotes:

  suppressWarnings({
    if(any(is.na(as.numeric(y_tick_names)))){
      y_tick_names[!is.na(as.numeric(y_tick_names))] <- ""
    }
  })

Its unclear why this design choice was chosen. I imagine it was to prevent numeric levels from being passed though, though I dont see why that would be cause for concern.

EDIT: After further inspection, I realized that code I removed was meant to handle no group_level being specified. By removing that code, we now get labels when none is supplied (which is not ideal). To fix this, I modified the section below:

  if (is.null(group_level) || length(group_level) != n) {
    if (!is.null(group_level) && length(group_level) != n) {
      warning("Argument group_level has wrong length and is ignored.")
    }
    group_level <- rep("", n) # used to be 1:n()
  }

Moving forward, both character interpretations of numeric columns, and actual numeric columns can be used:

Character of '10.00'

Value of 10

closes #24

[barrettk]

After further inspection, I realized that code I removed was meant to handle no group_level being specified. By removing that code, we now get labels when none is supplied (which is not ideal). Will adjust the PR description when I find a solution

[kyleam]

Character of '10'

The plots show "10.00". Probably just a typo in the PR description, but to make sure: was the character "10" or "10.00"?

[kyleam]

typo: chacter

[barrettk]

The plots show "10.00". Probably just a typo in the PR description, but to make sure: was the character "10" or "10.00"?

Correct, the specified character was "10.00", I just abbreviated it

[kyleam]

Correct, the specified character was "10.00", I just abbreviated it

Would you mind expanding it in the description? Abbreviating invites confusion in this case.

[kyleam]

After further inspection, I realized that code I removed was meant to handle no group_level being specified. By removing that code, we now get labels when none is supplied (which is not ideal). To fix this, I modified the section below:

Thanks for the analysis. I'm largely taking it at face value given I'm not familiar with this codebase, but that makes sense from what I can tell.

Reviewing this I've

• confirmed that the case reported in group_level not printing when character of a number #24 is resolved
• confirmed that the new test fails if you revert the code changes from this PR

From my view, this is good to go once the _snaps/ files are refreshed:
#25 (comment)

[barrettk]

@kyleam I noticed I was getting warnings about size being deprecated in the newest ggplot installation:

Warning (test-nsim-plot.R:23): Multiple simulations: Multiple simulations base test [PMF-PLOT-012]
Using `size` aesthetic for lines was deprecated in ggplot2 3.4.0.
ℹ Please use `linewidth` instead.

Taken from their latest release:

A linewidth aesthetic has been introduced and supersedes the size
aesthetic for scaling the width of lines in line based geoms. size will
remain functioning but deprecated for these geoms and it is recommended to
update all code to reflect the new aesthetic. For geoms that have both point
sizing and linewidth sizing (geom_pointrange() and geom_sf) size now
only refers to sizing of points which can leads to a visual change in old
code (@thomasp85, tidyverse/ggplot2#3672)

It seems this change was introduced in their latest release, and they plan on supporting the old method of size for a while. pmplots might be affected by this change as well, but im wondering when we should incorporate this change, given that we want to support older MPN snapshots, while being sure to make this change before ggplot stops supporting it.

If they were to install this package via install.packages() after size stops being supported, this could lead to issues since it would install the latest version of ggplot

[kyleam]

@kyleam I noticed I was getting warnings about size being deprecated in the newest ggplot installation:

I see you've found gh-26, which mentions this warning, so I'll follow up there (but yes, you're right that we need to consider backward compatibility).