R >= 3.5.0 dependence

[jonocarroll]

Introduced in e42a79500b399cadae7300d76226742424e5918c, but is this strictly necessary? It means the package can't be installed in earlier versions.

If it is strictly required, what is the 3.5.0 functionality this package relies on?

[adamhsparks]

I'm aware of the limitations.

I don't know what the change was between 3.4.3 and 3.5.0, but in the get_precis_forecast() function the column names are different between the versions of R and it breaks in older versions.

I tried this with a couple computers and had the same results. Also tried to make sure it wasn't just janitor that was causing issues. It wasn't that, definitely something in the version of R best I could find. If it's possible to track down and fix, I'm happy to drop the restriction back to 3.2.0.

[jonocarroll]

I might see if I can make a patch for a version-agnostic update. Cheers.

[adamhsparks]

I would welcome that!

Right now I don't have time to look into it.

[adamhsparks]

Actually, I'm fairly sure we could just check the R version and use the proper column names depending on that. I just didn't take the time to dig further. I probably should have, but I wanted to get this update on CRAN since I'd also received an e-mail from Kurt about missing dependencies.

[adamhsparks]

I've installed a Docker version with 3.4.3 to do some testing and see what I can figure out.

[adamhsparks]

Clicked wrong button. Didn't mean to close.

[adamhsparks]

I've pushed a fix to the devel branch. It checks the R version and modifies the colnames that are supplied accordingly. I'm baffled as to why the R version changes that but I've recreated it again tonight with different versions of R to be sure.

@jonocarroll, please let me know if this fixes the issue for you on 3.4.3. It passes CI tests with R 3.5.0.

[jonocarroll]

Tested devel branch in R 3.4.4 on Ubuntu/Pop!_OS 18.04 LTS.

  ══ testthat results  
  OK: 96 SKIPPED: 0 FAILED: 9
  1. Error: get_precis_forecast returns 19 columns (@test-get_precis_forecast.R#6) 
  2. Error: get_precis_forecast returns the forecast for ACT/NSW (@test-get_precis_forecast.R#59) 
  3. Error: get_precis_forecast returns the forecast for ACT/NSW (@test-get_precis_forecast.R#65) 
  4. Error: get_precis_forecast returns the forecast for NT (@test-get_precis_forecast.R#71) 
  5. Error: get_precis_forecast returns the forecast for SA (@test-get_precis_forecast.R#77) 
  6. Error: get_precis_forecast returns the forecast for TAS (@test-get_precis_forecast.R#83) 
  7. Error: get_precis_forecast returns the forecast for VIC (@test-get_precis_forecast.R#89) 
  8. Error: get_precis_forecast returns the forecast for WA (@test-get_precis_forecast.R#95) 
  9. Error: get_precis_forecast returns the forecast for AUS (@test-get_precis_forecast.R#101) 

I've pushed another patch to devel which I think fixes these. I now get just a single "server is not responding" error running devtools::test().

OK:       125
Failed:   1
Warnings: 0
Skipped:  0

All seem to pass when running the file manually, or withtest_file and all skip_on_crans turned off

OK:       31
Failed:   0
Warnings: 0
Skipped:  0

Please check on 3.5.0.

From what I can see:

• the if(getRversion() < "3.5.0") block did not have an else for the other condition (which I see you also fixed as I was looking at it)
• the < "3.5.0" branch of the block had "type_" (but not "units_") in the name, so didn't match what the column names actually were.

Given that "type" and "unit" are being inserted, this sounds like it might be a janitor thing, but I'd need to play with old and new versions of that to understand.

[adamhsparks]

Only one version of janitor will work here, but depending on the R version you get different results. @sfirke, do you have any ideas?

[adamhsparks]

Thanks, @jonocarroll, for cleaning up the mess I made. I was hurrying last night to patch it.

It passes tests on R 3.5.0 locally for me.

[sfirke]

Huh. janitor::clean_names() should behave the same regardless of R version. It depends on R at least v3.1.2. You all are seeing different behavior under 3.4 and 3.5?

[adamhsparks]

Yes, starting at line 239, https://github.com/ropensci/bomrang/blob/devel/R/get_precis_forecast.R#L239, you can see that I've had to check for the version of R. The colnames that are reported differs. Starting with R 3.5.0 the names are type_air_temperature_maximum_units_celsius, with type and units in the name. In prior versions the colnames appear as air_temperature_maximum_celsius.

[sfirke]

Would you be able to share the names that get passed into clean_names that are returning the different outputs under different versions of R? That will be easiest for me to investigate, a reprex of sorts.

[adamhsparks]

Gah. I'm a dork. This is a tidyr issue, not janitor.

[adamhsparks]

I'll go ahead and close this as I've incorporated a workaround in the code and opened an issue with tidyr.

No real issues here any longer. The next release of bomrang will only require R >= 3.2.0.