Fix: import reshape2 instead of reshape #17

Merged
merged 5 commits into from Jun 28, 2013

Conversation

Projects
None yet
3 participants
Contributor

jucor commented Jun 22, 2013

This should solve issue #15 , hope this helps :)

dmenne commented Jun 24, 2013

No, that's no enough. You have to run the test, for example variable_name is variable.name in reshape2

Contributor

jucor commented Jun 24, 2013

Great: so where are the unit tests that I can run? I can't find any.

dmenne commented Jun 24, 2013

Build the package which includes an example; there are no unit tests, but a full build/check for CRAN includes this.

Contributor

jucor commented Jun 24, 2013

Thanks. Weird: I had compiled the package, but didn't see the test. I probably had a no-check flag somewhere, or didn't pay attention. Will get back to it.

Contributor

jucor commented Jun 24, 2013

PS: I've asked on manipulatr mailing list for any info on broken backward compatibility between reshape and reshape2.

dmenne commented Jun 24, 2013

Instead wasting too many grey cells on the check-flags, use RStudio, create a package project, and Build/Check. I also suggest to explicitly check "roxygenize on Build and Reload", because that what one easily forgets.

The only other change is the _/. in melt.data.frame. But there is also the problem with Rhat and 1 chain, and the nasty histogram warnings.

Contributor

jucor commented Jun 24, 2013

Thanks Dieter. Looks like there's also cast() which has been renamed acast/dcast depending on the use. The warnings do not worry me so much: they are ugly indeed, but apparently can be fixed.

jucor added some commits Jun 24, 2013

@jucor jucor Replace cast() by dcast() from reshape2 4e76705
@jucor jucor Replace X1/X2 by Var1/Var2
reshape2's melt() names its columns Var1/Var2
while reshape used to name them X1/X2
6a991d0
Contributor

jucor commented Jun 24, 2013

Fixed the _/., the cast(), and a naughty melt() issue, too. Now the check passes smoothly, and doesn't raise any histogram warnings. Could you please give me examples to reproduce:

  • the histogram warnings
  • the problem of Rhat with 1 chain ?

If you add them as standalone files and pull-request on my fork jucor/ggmcmc , I will turn them into unit tests and work from there. Thanks!

dmenne commented Jun 24, 2013

You should see the histogram-warning when compiling the standard example. It comes from ggs_histogram/geom_histogram. I suggest the following changes, but cannot get it to work since I do not know the syntax (code below fails in geom_histogram).
Feel free to check:
http://stackoverflow.com/questions/17271968/different-breaks-per-facet-in-ggplot2-histogram

breaks = tapply(D$value,D$Parameter,function(x){ pretty(range(x), n = nclass.FD(x), min.n = 1) }) f <- ggplot(D, aes(x=value) ) + geom_histogram(breaks=breaks) + facet_wrap(~ Parameter, ncol=1, scales="free") return(f)

dmenne commented Jun 24, 2013

When trying to reproduce the 1-chain problem, I encountered another one which I remember had to do with some coda-related problems; I had already solved that once, but don't remember the details. See Issue #18. Sorry, I have to leave, cannot work on this one currently.

Contributor

jucor commented Jun 24, 2013

Thanks for your help Dieter! I'll fix what I can/need, and contact @xfim to see if he can pull.

Contributor

jucor commented Jun 24, 2013

The problem with the multiple binwidths appears to be that breaks can only specify common breaks for all variables. There are a few workarounds, but the desagreement of the warning is small enough and the effort to high for me to push further :-) I suggest filling a separate issue.

Contributor

jucor commented Jun 24, 2013

Bottom line: the port to reshape2 is done here. The Rhat with 1 chain is also a separate problem (that I'll be glad to look into, too), but at least this issue is ripe for merging. Emailing @xfim. Thanks for the input Dieter!

xfim merged commit 6a991d0 into xfim:master Jun 28, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment