Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

unintended behavior in melt? #30

Open
davharris opened this Issue · 4 comments

2 participants

@davharris

Hi Hadley, thanks for writing such a cool package.

I was looking through the source for melt.array today, and I'm confused about its handling of character vectors. I thought I'd file an issue to make sure it's behaving as you intended.

The source code and comments seem to indicate that you didn't want to return factors (e.g. stringsAsFactors = FALSE in line 120), but I'm getting factors anyway with the following example (using version 1.2.2):

a = matrix(1:9, nrow = 3,dimnames = list(letters[1:3], LETTERS[1:3]))

str(melt(a))

I've tracked down where the conversion to factors is occurring, in case you'd to change it or add it as an option for the user. If you're interested, I'd be happy to write a pull request.


Currently, you run the following function on each dimname:

  var.convert <- function(x) if(is.character(x)) type.convert(x) else x

This calls type.convert, which includes as.is = FALSE by default. This means that character vectors that can't be coerced to other classes will be converted to factors, and there won't be any more strings in dn, which is a temporary label list. As a result, the stringsAsFactors = FALSE argument in the next line doesn't do anything, because there are no strings left to convert.

There seems to be a similar issue with melt.data.frame, but I haven't looked at it as closely.


It seems like it would be nice to have the option to return strings (or possibly to change the default behavior, if that's what you'd prefer). Could I submit a pull request to add this feature?

Thanks again!

@hadley
Owner

Yup, that's a bug, and I'd happily accept a pull request that fixed it and added a few more tests.

@davharris

Great. I have a preliminary fix here. I'll start writing more unit tests next; all the existing ones pass.

Before I submit the pull request, I thought I'd ask--what would you like the default for as.is to be? TRUE seems to have been your original intention (and is probably the least annoying option in most cases that it matters), but it might break existing workflows that rely on the current behavior.

Thanks again for making such a great tool!

@hadley
Owner

Default should definitely be as.is = TRUE. Would love a PR with some tests.

@hadley
Owner

But you're right that it's too late to change the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.