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

Vignette row names #34

Merged
merged 2 commits into from Dec 22, 2016
Merged

Vignette row names #34

merged 2 commits into from Dec 22, 2016

Conversation

@wibeasley
Copy link
Contributor

@wibeasley wibeasley commented Dec 22, 2016

Thanks for creating this package, @jtilly. I found the documentation very helpful too. I'm just learning the concepts and terminology, so my feelings won't be hurt if you reject this PR.

In the two-sided market section of the vignette, the row names for a utility matrix are Woman 1 ...Woman 3. But if I understand correctly, the rows of the preference matrix represent something different. Are the terms Preference 1...Preference 3 more representative?

The two modifications to the Rmd file are the only real changes I made. The others appear to be the result of how Rcpp auto-generates some code differently.

@coveralls
Copy link

@coveralls coveralls commented Dec 22, 2016

Coverage Status

Coverage decreased (-0.01%) to 99.54% when pulling b06f8ae on wibeasley:vignette-row-names into 195b87f on jtilly:master.

@jtilly
Copy link
Owner

@jtilly jtilly commented Dec 22, 2016

@jtilly jtilly merged commit b06f8ae into jtilly:master Dec 22, 2016
1 of 2 checks passed
1 of 2 checks passed
coverage/coveralls Coverage decreased (-0.01%) to 99.54%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jtilly added a commit that referenced this pull request Dec 22, 2016
@wibeasley
Copy link
Contributor Author

@wibeasley wibeasley commented Dec 22, 2016

If there's something else you'd like it named, just tell me. It's still set up on my computer.

@jtilly
Copy link
Owner

@jtilly jtilly commented Dec 22, 2016

Thanks, I already managed to sneak in a little change before I merged.

I'm now going with

##         cols
## rows     Woman 1 Woman 2 Woman 3
##   Rank 1       3       1       3
##   Rank 2       2       3       2
##   Rank 3       1       2       1
@wibeasley
Copy link
Contributor Author

@wibeasley wibeasley commented Dec 22, 2016

oh good, I like the brevity of "Rank X" better too.

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.