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

bug in make_expectation #52

Closed
wants to merge 1 commit into from
Closed

bug in make_expectation #52

wants to merge 1 commit into from

Conversation

@scottkosty
Copy link

@scottkosty scottkosty commented Nov 10, 2012

Related to #24. make_expectation now works with more types of
objects. For example, a data.frame previously lost its data.frame
attribute and was treated as a generic list. The test generated by
make_expectation would thus fail if run.

This patch is ugly. However, the current version of make_expectation does not work
on data.frames and worse it generates bad tests without giving an error. By bad,
I mean that the tests will fail. To see this, use the current version of make_expectation
on a data.frame.

Related to Issue #24. make_expectation now works with more types of
objects. For example, a data.frame previously lost its data.frame
attribute and was treated as a generic list. The test generated by
make_expectation would thus fail if run.
@hadley
Copy link
Member

@hadley hadley commented Nov 12, 2012

Would you mind providing a few test cases? That'd help be think about potentially more elegant solutions.

@scottkosty
Copy link
Author

@scottkosty scottkosty commented Nov 12, 2012

Sure. I think that the current version of make_expectation will provide an incorrect test for any data.frame object. I put an example in the commit that did not work with the previous version of make_expectation but that does work with the patched version:
make_expectation(subset(mtcars, mpg > 25))
Another example would be: make_expectation(mtcars)
Perhaps the simplest example is: make_expectation(data.frame(x = 2))

@hadley
Copy link
Member

@hadley hadley commented Mar 20, 2013

I think this might actually be a bug in R: substitute(f(x), list(x = data.frame(y = 1))) does not print correctly.

@hadley hadley closed this in 7682115 Mar 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants