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

Support R variables with . and data.frames with $ #11

Closed
wants to merge 1 commit into from

Conversation

KentonWhite
Copy link

I was incorporating whisker into ProjectTemplate at a user request and was having problems handling variables that have '.' in their name as well as referencing columns of a data.frame using the '$' notation.

Sure enough, in the code there was a TODO about this!

The code looks like it is splitting variables on '.' to create nested object method calls. E.g if I pass in "foo.bar" the variable name is foo and the method name to call on foo is bar. To the best of my knowledge, method calls on objects in R don't use '.'.

The common use cases that I'm aware of are using '$' to reference a column of data on a data.frame and using '@' to access a slot on an object. I've updated the code to handle variable names with '.' and to handle the '$' on data.frames. E.g. if I pass in "foo$bar" it will take the variable name to be foo and the column to return as bar. If I pass "foo.bar" it will take the variable name to be foo.bar.

I didn't make changes for the slots, but made a TODO note.

@edwindj
Copy link
Owner

edwindj commented Feb 26, 2014

Thanks for your addition! But it is a difficult one:

whisker conforms to the mustache language: it is programming language agnostic.
Changing "." into "$" lets whisker fail many compliance tests:

library(devtools)
test_package("whisker")

I suggest that we implement it as an option:
the default is mustache compliant, but you can specifiy, via options, or whisker.render(..., use_dollar=TRUE)

Would that be an option?

Best

@KentonWhite
Copy link
Author

Ah yes! I see now on the mustache spec for interpolation the using '.' as the context sigil is written into the spec. In this (discussion thread)[https://github.com/mustache/spec/issues/52] there is mention of the difficulty of writing about the context stack in an implementation agnostic way. The gist is that context stack implementation should be left to the implementors.

I'll ask at mustache/spec for clarification.

In the mean time, why not add whisker.render(..., context_sigil = '$'). This can default to the '.' context sigil but also leaves it open for people wanting to access object slots with a '@' context sigil.

@KentonWhite
Copy link
Author

Got some feedback from other Mustache implementors. Specifically he says

However, the spec does not enforce that a single input (data + template) should render the same in all implementations. The weak spot is the way booleans are handled (see issue #4). In some languages, 0 (zero) would be truthy, and in other languages, the same value would be falsey.

So whisker should free to define interpolation to be the natural usage for R. In fact, other Mustache implementors encourage this!

Looking at test interpolation.R, the tests for context interpolation represent the data structure person = list(name = "Joe") as person.joe. In my opinion, this is not intuitive. If I have person = list(name = "Joe") loaded in my environment, person.name gives Error: object 'person.name' not found. This requires me to know a "hidden" syntax to make my Mustache templates work in whisker.

I've played around with the tests, and using '$' instead of '.' works except for the method chaining part of the spec since R doesn't allow for chaining of data frames.

Maybe instead of having options context_sigil or user_dollar there can be an option strict. Using strict will interpret the template strictly according to the spec. Otherwise the template will be interpretted using a more natural R syntax. The default can be FALSE and set to TRUE for testing against the spec. Out of the box whisker would support interpolation along the lines of what I think users would expect. And for those really concerned about reusing templates from other languages, and investing the time to make R data that complies, they can set strict = TRUE.

@groue
Copy link

groue commented Feb 27, 2014

To clarify: I'm not a Mustache maintainer. I'm a template engine implementor, like you.

@KentonWhite
Copy link
Author

@groue sorry for the misunderstanding. I've updated to comment to avoid any further confusion.

@groue
Copy link

groue commented Feb 28, 2014

@KentonWhite You're welcome. Actually Mustache is no longer maintained: the people who are responsible of the specification repository haven't posted any answer to any comment in at least a year now.

@ctbrown
Copy link

ctbrown commented Mar 4, 2014

Interesting thread. I think that this is a problem with R that I have has some grumblings. The allowed use of . in name was a rather poor decision by the R/S development team when so many other languages were using it as a (dereference) operator.

@edwindj
Copy link
Owner

edwindj commented Mar 4, 2014

I'm happy to add $ support, but the default (for the moment) will be the ..
This is because it is a breaking change and some packages depend on it.
(e.g. rMaps and rChart use whisker to generate html and javascript).

edwindj added a commit that referenced this pull request Mar 4, 2014
"strict" to render. Setting it to FALSE allows for "." in names
and use "$" for splitting which is a more natural R syntax. Thanks to @KentonWhite
(issue #11)
@KentonWhite
Copy link
Author

Thanks! Closing since changes merged into master.

@KentonWhite KentonWhite closed this Mar 5, 2014
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.

4 participants