div III feedback

[mariumtapal]

1. Passes install_github: ✅
2. Documentation builds: ✅
3. Passes R CMD check: ✅

0 errors ✓ | 0 warnings ✓ | 0 notes ✓

@beanumber

[beanumber]

• The title of your vignette is "Vignette"
• I see some warnings on some of your tests:

  
  Warning (test-data.R:33:3): Checking read_ftirs_file
  Columns renamed to `wavenumber`, `absorbance`. Please make sure these
        labels match the contents of the columns.
  Backtrace:
  1. testthat::expect_warning(read_ftirs_file("test_samples/FISK-270.0.csv"))
    at test-data.R:33:2
  7. ftirsr::read_ftirs_file("test_samples/FISK-270.0.csv")

Warning (test-data.R:121:3): Checking interpolation collapsing to unique 'x' values Backtrace:

1. testthat::expect_warning(...) at test-data.R:121:2
2. ftirsr::interpolate_ftirs(...)
3. stats::approx(...) at plsr/R/interpolate.R:24:2
4. stats:::regularize.values(x, y, ties, missing(ties), na.rm = na.rm)

• I think you need to re-export the tidyr::pivot_longer() generic if you want this to work properly.

 > pivot_longer(greenland)
Error in pivot_longer(greenland) : could not find function "pivot_longer"

• Excellent use of S3 classes!

[hartlegr]

@beanumber Thank you for the feedback! These two warnings are expected/because two of the tests are expect_warning() and intentionally doing something that throws a warning. Is there a way to still expect_warning without suppressing the warning messages for those two examples? We tried that, but then it failed the expect_warning test...

[hartlegr]

What do you mean by re-export tidyr::pivot_longer()? Where would this be done?

[beanumber]

What I think is happening is that your pivot_longer() method is exported by your package, but the pivot_longer() generic is not. Therefore, until you library(tidyr), the generic is not visible, and thus the error. Once you library(tidyr), the error goes away.

So I think one solution is for you to re-export the pivot_longer() generic from tidyr in your package.

See examples in dplyr.

[hartlegr]

This would not be within our function code, but another file called reexport-pivot_longer.r? And this isn't solved by importing tidyr::pivot_longer in our function code because the function tidyr::pivot_longer is needed when we call ftirsr::pivot_longer?

[beanumber]

You can put it wherever you like, but it has to go in the R/ folder.

Correct, it is not solved by importing tidyr::pivot_longer(), it's solved by exporting tidyr::pivot_longer().