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 for alliterative adjective animals #6

Merged
merged 4 commits into from Jul 18, 2019
Merged

Conversation

richfitz
Copy link
Member

Fixes #5, as discussed on twitter https://twitter.com/grrrck/status/1151475544325873664

Allows generation of adjective animals like classy_cow, skilled_spider, etc

@codecov-io
Copy link

codecov-io commented Jul 17, 2019

Codecov Report

Merging #6 into master will increase coverage by 1.21%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #6      +/-   ##
========================================
+ Coverage   98.78%   100%   +1.21%     
========================================
  Files           7      7              
  Lines         247    263      +16     
========================================
+ Hits          244    263      +19     
+ Misses          3      0       -3
Impacted Files Coverage Δ
R/adjective_animal.R 100% <100%> (ø) ⬆️
R/proquint.R 100% <100%> (+1.82%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1444d9...27efcea. Read the comment docs.

@richfitz richfitz requested a review from weshinsley July 18, 2019 10:48
@@ -15,6 +15,12 @@
#' first element will apply to the adjectives (all of them) and the
#' second element will apply to the animals.
#'
#' @param alliterate Produce "alliterative" adjective animals (e.g.,
#' \code{hessian_hampster}). Note that this cannot provide an equal
#' probability of any particuilar combination because it forces a
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo - hamster.
:-D

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤣 I just got a call from @richfitz: “I mean, I always spell hamster with a P, he has no right to criticize me!”

## We can generate alliterative ids by either rejection sampling
## (which will be hard with multiple adjectives) or by doing a
## weighted sample of letters and then working with each letter
## separately. Do do this properly we should compute the number of
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do this

## separately. Do do this properly we should compute the number of
## distinct combinations and avoid duplications but that seems
## excessive for this and is only an issue if the number of adjectives
## is greater than one.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion point: when I tested this:

> ids::adjective_animal(alliterate = TRUE, max_len=3, n_adjectives=3)
[1] "fat_fat_fit_fly"

I almost always got adjectives beginning with F - I guess because there are more 3-lettered, um, F-words, than there are others. (a=2, b=2, c=3, d=2, e=1, f=7, g=1, h=2, i=3, j=1, k=1, l=2, m=2, n=1, o=3, p=0, r=2, s=4, t=3, u=0, v=0, w=3, x=0, y=0, z=0).

This is a weaknes of the adjective set, and indeed, the English language, but not of the algorithm I think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a future extension could include a parameter for forbid_duplicated_adjectives = TRUE

@@ -1,6 +1,10 @@
# ids 1.1.0 (2017-05-22)
# ids 1.1.0 (unreleased)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the "unreleased" gets updated at some time?

ids(n, vals = m[start, , drop = TRUE], style = style)
}

gen <- function(n) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take n=1 as an argument, so you can drop the closure in 116-118.

@richfitz richfitz requested a review from weshinsley July 18, 2019 15:04
@weshinsley weshinsley merged commit 2390dab into master Jul 18, 2019
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.

Feature Request: Alliterative IDs
4 participants