Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use data.frame() instead of melt() in div_profile_plot; other minor fixes #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tdhock
Copy link

@tdhock tdhock commented Dec 19, 2024

in Rdatatable/data.table#6670 we see that checking hilldiv from CRAN (1.5.1) using new data.table from github master, results in a new check issue: error in example(div_profile_plot), which does not happen using data.table from CRAN (1.16.4).

This PR is my attempt to fix hilldiv so that it works with the new version of data.table, which we plan to release in the next month or so.

I began by cloning hilldiv from github and trying to reproduce the same error on Rdatatable/data.table#6670 (using hilldiv from CRAN).
I noticed there were a bunch of minor R CMD check issues which I have fixed in this PR.
Now using data.table master I get the following error when checking this PR version of hilldiv

* checking examples ... [29s] ERROR
Running examples in 'hilldiv-Ex.R' failed
The error most likely occurred in:
> base::assign(".ptime", proc.time(), pos = "CheckExEnv")
> ### Name: pair_dis_plot
> ### Title: Pairwise dissimilarity plot
> ### Aliases: pair_dis_plot
> ### Keywords: diversity hill numbers pairwise partitioning
> 
> ### ** Examples
> 
> data(bat.diet.otutable)
> data(bat.diet.tree)
> data(bat.diet.hierarchy)
> pairdisres <- pair_dis(bat.diet.otutable,qvalue=0,hierarchy=bat.diet.hierarchy)
> pair_dis_plot(pairdisres$L2_CqN,hierarchy=bat.diet.hierarchy,type="NMDS")
Error in pair_dis_plot(pairdisres$L2_CqN, hierarchy = bat.diet.hierarchy,  : 
  Distance matrix and hierarchy table do not match.

above is not the same error as we get when checking hilldiv from CRAN.

below I try to check hilldiv from this PR, using data.table from CRAN, and I get the same error.

* checking examples ... [29s] ERROR
Running examples in 'hilldiv-Ex.R' failed
The error most likely occurred in:
> base::assign(".ptime", proc.time(), pos = "CheckExEnv")
> ### Name: pair_dis_plot
> ### Title: Pairwise dissimilarity plot
> ### Aliases: pair_dis_plot
> ### Keywords: diversity hill numbers pairwise partitioning
> 
> ### ** Examples
> 
> data(bat.diet.otutable)
> data(bat.diet.tree)
> data(bat.diet.hierarchy)
> pairdisres <- pair_dis(bat.diet.otutable,qvalue=0,hierarchy=bat.diet.hierarchy)
> pair_dis_plot(pairdisres$L2_CqN,hierarchy=bat.diet.hierarchy,type="NMDS")
Error in pair_dis_plot(pairdisres$L2_CqN, hierarchy = bat.diet.hierarchy,  : 
  Distance matrix and hierarchy table do not match.

@tdhock
Copy link
Author

tdhock commented Dec 19, 2024

The example failure above is un-related to data.table, which can be seen by comparing the output below (using data.table from CRAN).

hoct2726@DINF-THOCK-01A MINGW64 ~/R
$ R -e 'packageVersion("data.table");example("div_profile_plot",package="hilldiv")'

R Under development (unstable) (2024-11-11 r87319 ucrt) -- "Unsuffered Consequences"
Copyright (C) 2024 The R Foundation for Statistical Computing
Platform: x86_64-w64-mingw32/x64

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> packageVersion("data.table");example("div_profile_plot",package="hilldiv")
[1] '1.16.4'
Registered S3 methods overwritten by 'FSA':
  method       from
  confint.boot car 
  hist.boot    car 

dv_pr_> data(bat.diet.otutable)

dv_pr_> data(bat.diet.hierarchy)

dv_pr_> #One sample example
dv_pr_> bat.diet.sample <- bat.diet.otutable[,1]

dv_pr_> profile.onesample <- div_profile(count=bat.diet.sample,qvalues=seq(from = 0, to = 5, by = (0.1)))

dv_pr_> div_profile_plot(profile.onesample)

dv_pr_> #Multiple samples
dv_pr_> profile.multiplesamples <- div_profile(bat.diet.otutable)

dv_pr_> div_profile_plot(profile.multiplesamples)

dv_pr_> #Multiple groups (gamma diversity)
dv_pr_> profile.multiplegroups <- div_profile(bat.diet.otutable,hierarchy=bat.diet.hierarchy,level="gamma")

dv_pr_> div_profile_plot(profile.multiplegroups)
Warning messages:
1: In melt.default(profile) :
  The melt generic in data.table has been passed a matrix and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is superseded and is no longer actively developed, and this redirection is now deprecated. To continue using melt methods from reshape2 while both libraries are attached, e.g. melt.list, you can prepend the namespace, i.e. reshape2::melt(profile). In the next version, this warning will become an error.
2: In melt.default(profile) :
  The melt generic in data.table has been passed a matrix and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is superseded and is no longer actively developed, and this redirection is now deprecated. To continue using melt methods from reshape2 while both libraries are attached, e.g. melt.list, you can prepend the namespace, i.e. reshape2::melt(profile). In the next version, this warning will become an error.

above is a warning.

below it is an error (using data.table dev version)

(base) �]0;MINGW64:/c/Users/hoct2726/R�
hoct2726@DINF-THOCK-01A MINGW64 ~/R
$ R -e 'packageVersion("data.table");example("div_profile_plot",package="hilldiv")'

R Under development (unstable) (2024-11-11 r87319 ucrt) -- "Unsuffered Consequences"
Copyright (C) 2024 The R Foundation for Statistical Computing
Platform: x86_64-w64-mingw32/x64

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> packageVersion("data.table");example("div_profile_plot",package="hilldiv")
[1] '1.16.99'
Registered S3 methods overwritten by 'FSA':
  method       from
  confint.boot car 
  hist.boot    car 

dv_pr_> data(bat.diet.otutable)

dv_pr_> data(bat.diet.hierarchy)

dv_pr_> #One sample example
dv_pr_> bat.diet.sample <- bat.diet.otutable[,1]

dv_pr_> profile.onesample <- div_profile(count=bat.diet.sample,qvalues=seq(from = 0, to = 5, by = (0.1)))

dv_pr_> div_profile_plot(profile.onesample)

dv_pr_> #Multiple samples
dv_pr_> profile.multiplesamples <- div_profile(bat.diet.otutable)

dv_pr_> div_profile_plot(profile.multiplesamples)
Error in melt.default(profile) : 
  The melt generic in data.table has been passed a matrix and will attempt to redirect to the relevant reshape2 method; please note that reshape2 is superseded and is no longer actively developed, and this redirection is now deprecated. To continue using melt methods from reshape2 while both packages are attached, e.g. melt.list, you can prepend the namespace, i.e. reshape2::melt(profile). In the next version, this warning will become an error.
Calls: example ... melt -> melt.default -> stopf -> raise_condition -> signal
Execution halted
(base) �]0;MINGW64:/c/Users/hoct2726/R�

so this confirms that the issue is reproducible using the hilldiv code in this PR.

@tdhock tdhock marked this pull request as ready for review December 19, 2024 14:32
@tdhock
Copy link
Author

tdhock commented Dec 19, 2024

Using this PR (b6b7416) I no longer get an error from running that example:

hoct2726@DINF-THOCK-01A MINGW64 ~/R/hilldiv (remove-data-table-melt)
$ R -e 'packageVersion("data.table");example("div_profile_plot",package="hilldiv")'

R Under development (unstable) (2024-11-11 r87319 ucrt) -- "Unsuffered Consequences"
Copyright (C) 2024 The R Foundation for Statistical Computing
Platform: x86_64-w64-mingw32/x64

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> packageVersion("data.table");example("div_profile_plot",package="hilldiv")
[1] '1.16.99'
Registered S3 methods overwritten by 'FSA':
  method       from
  confint.boot car
  hist.boot    car

dv_pr_> data(bat.diet.otutable)

dv_pr_> data(bat.diet.hierarchy)

dv_pr_> #One sample example
dv_pr_> bat.diet.sample <- bat.diet.otutable[,1]

dv_pr_> profile.onesample <- div_profile(count=bat.diet.sample,qvalues=seq(from = 0, to = 5, by = (0.1)))

dv_pr_> div_profile_plot(profile.onesample)

dv_pr_> #Multiple samples
dv_pr_> profile.multiplesamples <- div_profile(bat.diet.otutable)

dv_pr_> div_profile_plot(profile.multiplesamples)

dv_pr_> #Multiple groups (gamma diversity)
dv_pr_> profile.multiplegroups <- div_profile(bat.diet.otutable,hierarchy=bat.diet.hierarchy,level="gamma")

dv_pr_> div_profile_plot(profile.multiplegroups)
>

@anttonalberdi can you please review, merge, fix the other R CMD check issues, and then submit to CRAN?
We would like to submit data.table to CRAN soon, and CRAN requires that our updates do not cause new errors in dependencies like hilldiv.
This PR would fix that issue, by making hilldiv no longer import data.table.
Thanks in advance!

@tdhock tdhock changed the title make hilldiv work with new data.table use data.frame() instead of melt() in div_profile_plot; other minor fixes Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: github UI points out missing terminal newlines in this & .gitignore

@@ -37,7 +37,6 @@ importFrom(RColorBrewer,brewer.pal)
importFrom(ape,cophenetic.phylo)
importFrom(ape,drop.tip)
importFrom(ape,is.ultrametric)
importFrom(data.table,melt)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe worth noting that, IINM, the trivial fix here is to just change this line to importFrom(reshape2, melt).

but as {reshape2} is retired, I agree your approach to use row()/col() directly makes more sense.

@MichaelChirico
Copy link

Great work fixing the class(x) == y issue! For OP's future reference, this can be caught by lintr::class_equals_linter():

lintr::lint_package(linters = lintr::class_equals_linter())

I ran lint_package() to find any other common mistakes, fixed in #5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants