Reduce number of suggested dependencies

[jangorecki]

Ideally we want suggested dependency to be packages that we call from data.table functions (conditionally of course). Packages that are only used in tests will fit better into our integration tests workflow (test.data.table(with.other.packages=TRUE)) thus should be moved from ./DESCRIPTION to ./inst/tests/tests-DESCRIPTION.
Current Suggests:

• bit64 - supported data type in data.table
• curl - used in fread to access https/ftps, probably needed on R 3.1.0
• knitr - used to build vignettes
• xts - used in as.data.table.xts
• nanotime - used in fwrite
• zoo - used in as.data.table.xts - it is dependency for xts anyway
• chron - removed in as.chron methods removed. Not sure why we had them anyway. #2831
• reshape2
• testthat (>= 0.4) - removed in remove testthat #2909
• GenomicRanges - removed in move GenomicRanges to integration tests #2726

[MichaelChirico]

I agree with the instinct to be minimal regarding package coverage, but IINM you just described the Imports field. I think you would do well to ellaborate on the distinction between Suggests and Imports as you see it, or to advocate for the removal of the Suggests field altogether

…

On Thu, Apr 12, 2018, 10:22 PM Jan Gorecki ***@***.***> wrote: Ideally we want suggested dependency to be packages that we call from data.table functions. Packages that are only used in tests will fit better into our integration tests workflow ( test.data.table(with.other.packages=TRUE) thus should be moved from ./DESCRIPTION to ./inst/tests/tests-DESCRIPTION. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2740>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHQQdcaMyr_jrLaBD8zJHAUWDDjCaFS9ks5tn2MggaJpZM4TR4wE> .

[mattdowle]

I already reduced the number of Suggests from 21 to 10 in PR #2671.
In that PR I added reasons why the remaining Suggests remain, near the top of tests.Rraw, here. In that context I'm not sure what the tick boxes mean in the first comment here above in this issue. Let's try and keep those reasons in one place, in the code at the top of tests.Rraw.
It would be good to remove GenomicRanges in Suggests for the reasons you state there, so let's keep #2726 open. But otherwise, we're good for now I think.

[jangorecki]

@mattdowle

• reshape2 has melt generic so we can safely remove call to reshape2 there, it will dispatch to next method from reshape2. dcast is not generic in reshape2, we could make PR adding that. Then should be fine to move reshape2 integration tests.
• testthat - this is not covered in tests.Rraw, maybe it is a matter of calling test_check() from other.Rraw...

Both those packages has multiple recursive dependencies.

[mattdowle]

@jangorecki

• Yes reshape2 would be good to revisit eventually. But low priority for now. PR was already made by @arunsrinivasan to make dcast generic, iirc.
• testthat is needed for the two files in /tests/testthat; e.g. S4 tests. Those tests would need to be rewritten to use our test().

Both so low priority that it's better this issue is closed.

[jangorecki]

As discussed it make sense to remove a little bit more of suggested dependencies, as we actually not suggesting them, we are just using them in tests. testthat is pretty heavy.

[mattdowle]

Yes I suggested yesterday we could remove testthat in this release.