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

Some functions to go along the db #25

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ColinFay
Copy link

@ColinFay ColinFay commented Oct 1, 2018

Here are some funs to play with the db:

sweary_is("shit", "en")
[1] TRUE

vec <- c("Yeah, nigga, we're still fucking with you, Still waters run deep ,Still Snoop Dogg and D.R.E. '99, nigga, guess who's back; Still, still doin' that shit, Andre? Oh for sho' Yeah! Check me out")

sweary_n(vec, "en")
[1] 3

sweary_pct(vec, "en")
[1] 8.823529

sweary_any(vec, "en")
[1] TRUE

sweary_censor(vec, "en")
[1] "Yeah, ***, we're still ***ing with you, Still waters run deep ,Still Snoop Dogg and D.R.E. '99, ***, guess who's back; Still, still doin' that ***, Andre? Oh for sho' Yeah! Check me out"

R/detect.R Outdated
check_is_lang(lang_code)
splitted <- tokenize_words(vec, simplify = TRUE)
n <- sweary_n(vec, lang_code)
if (n !=0){
Copy link
Owner

Choose a reason for hiding this comment

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

Add spaces so that the code is clean if (n != 0) {.

Copy link
Author

Choose a reason for hiding this comment

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

Done

context("test-detect")

dre <- c("Yeah, nigga, we're still fuckin' with you Still waters run deep Still Snoop Dogg and D.R.E. '99, nigga, guess who's back Still, still doin' that shit, Andre? Oh for sho' Yeah Check me out It's still Dre Day nigga, AK nigga Though I've grown a lot, can't keep it home a lot ‘Cause when I frequent the spots that I'm known to rock You hear the bass from the truck when I'm on the block Ladies they pay homage, but haters say Dre fell off How? Nigga, my last album was The Chronic They wanna know if he still got it They say rap's changed, they wanna know how I feel about it (If you ain't up on thangs) Dr. Dre is the name, I'm ahead of my game Still puffin' my leaves, still fuck with the beats Still not lovin' police (Uh-uh) Still rock my khakis with a cuff and a crease (For sho') Still got love for the streets, reppin' 213 (For life) Still the beats bang, still doin' my thing Since I left ain't too much changed, still I'm representin' for them gangstas all across the world (Still) Hittin' them corners in them lo-lo's, girl Still takin' my time to perfect the beat And I still got love for the streets, it's the D.R.E. I'm representin' for them gangstas all across the world (Still) Hittin' them corners in them lo-lo's, girl Still takin' my time to perfect the beat And I still got love for the streets, it's the D.R.E. Since the last time you heard from me I lost some friends Well, hell, me and Snoop, we dippin' again Kept my ear to the streets, signed Eminem He's triple platinum, doin' 50 a week Still, I stay close to the heat And even when I was close to defeat, I rose to my feet My life's like a soundtrack I wrote to the beat Treat rap like Cali weed: I smoke 'til I sleep Wake up in the A.M., compose a beat I bring the fire 'til you're soakin' in your seat It's not a fluke, it's been tried, I'm the truth Since 'Turn Off the Lights' from the World Class Wreckin Cru I'm still at it, after mathematics In the home of drive-bys and ak-matics Swap meets, sticky green, and bad traffic I dip through, then I get skin, D.R.E. I'm representin' for them gangstas all across the world (Still) Hittin' them corners in them lo-lo's, girl Still takin' my time to perfect the beat And I still got love for the streets, it's the D.R.E. I'm representin' for them gangstas all across the world (Still) Hittin' them corners in them lo-lo's, girl Still takin' my time to perfect the beat And I still got love for the streets, it's the D.R.E. It ain't nothin' but more hot shit Another classic CD for y'all to vibe with Whether you're coolin' on the corner with your fly bitch Laid back in the shack, play this track I'm representin' for the gangstas all across the world Still (Hittin' them corners in them lo-lo's, girl) I'll break your neck, damn near put your face in your lap Niggas try to be the king, but the ace is back (So if you ain't up on thangs) Dr. Dre be the name, still runnin' the game Still got it wrapped like a mummy Still ain't trippin', love to see young blacks get money Spend time out the hood, take they moms out the hood Hit my boys off with jobs, no more livin' hard Barbeques every day, drivin' fancy cars Still gon' get mine regardless (Still) I'm representin' for them gangstas all across the world (Still) Hittin' them corners in them lo-lo's, girl Still takin' my time to perfect the beat And I still got love for the streets, it's the D.R.E. I'm representin' for them gangstas all across the world (Still) Hittin' them corners in them lo-lo's, girl Still takin' my time to perfect the beat And I still got love for the streets, it's the D.R.E. I'm representin' for them gangstas all across the world (Still) Hittin' them corners in them lo-lo's, girl Still takin' my time to perfect the beat And I still got love for the streets, it's the D.R.E.b Right back up in your mothafuckin' ass 9-5 plus four pennies! Add that shit up D.R.E. right back up on top of thangs Smoke some with your Dogg No stress, no seeds, no stems, no sticks! Some of that real sticky icky icky Ooh wee! Put it in the air! Well, you's a fool Dr.")

Copy link
Owner

Choose a reason for hiding this comment

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

Do we need two newlines here? I'm not sure if this is a convention or not.

Copy link
Author

Choose a reason for hiding this comment

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

Nop, not a convention, mistake. Changed.

@pdrhlik
Copy link
Owner

pdrhlik commented Oct 2, 2018

Hi @ColinFay! Thanks for the PR, it's awesome!

You added some things that were definitely on the way. You just pushed it in the right direction :-)

I have a few comments on some of the commits.

  1. How exactly is devtools_history.R used? I haven't seen it before and I don't know its purpose although I suspect it.
  2. The main 5 functions are great. I'm only not sure about the naming convention you started. I'd probably prefer to be more explicit. Mainly because it might be used in long pipes with different commands. If we are explicit at the cost of longer function names, the code will be more easily readable. So I suggest naming it like n_swearwords instead of sweary_n.
  3. Could you add newlines to every file?
  4. Could you remove the empty lines between function documentations and bodies?
  5. attempt looks great. I'd like the package to be with as less dependencies as possible. I am using quite a lot of packages and none of them forced me to install attempt yet :-) Could you persuade me?
  6. Your tests don't pass. Check the travis ouput. Sneak peek at the result is below.
en_swear_words <- get_swearwords("en")
Error in .x %in% swear_lang : object 'swear_lang' not found
Calls: get_swearwords ... check_is_lang -> stop_if_not -> <Anonymous> -> %in%

An offtopic question at the end. How did you come across this project?

@ColinFay
Copy link
Author

ColinFay commented Oct 2, 2018

Hey,

So, I'll guess I'll answer point by point:

  1. devtools_history is a file I create to keep trace of all the dev related action I've performed. That way when you receive you have a direct file to refer to for all the things that could otherwise be run in the console. Feel free to drop it if you want :)

  2. No pblm :) Here are the new function names I suggest : is_swearword, pct_swearword, n_swearword, any_swearword, censor_swearword. I've kept them singular, as it seems to me important to keep some kind of consistency between function names. Tell me if this sounds ok to you.

  3. Done

  4. Done

  5. {attempt} is a package of mine :) Low on dependencies (just has {rlang}, which in return has zero dep), so the dependency tree is pretty small. I never remember how to use base::stopifnot, and when I do, it seems a little bit cumbersome to write something to catch if it's an error, and then writing a use message. This is what is done directly in attempt::stop_if_not : test, and if the condition is not met, the error message specified is returned. Let me know if you'd prefer me to rewrite the check function, no pblm.

  6. Arf, noob error: I forgot to push a file :/ I have a vector called swear_lang that contains the unique of the languages. It now appears to me that it is only called once in the check function, so I've put the unique there instead.

And for the last question, I stumbled upon your repo as I was doing some text mining, and was looking for a "bad word labeller". I coded some functions upon the package, so it seemed natural to me to send these one to you.

@pdrhlik
Copy link
Owner

pdrhlik commented Oct 4, 2018

Hi @ColinFay, sorry for the delayed reply.

Point by point to you again.

  1. I would drop it for now if you don't mind :-)
  2. This is a lot better. I definitely want the function names to be consistent. I'm just thinking that some of them might be plural. I would only keep is_swearword singular and rename others to include the plural "s" at the end. It makes sense even if you take a look at the function's arguments. is_swearword works with a single word but all of the others work with vectors. Does it make sense to you too?
  3. OK
  4. OK
  5. I've noticed {attempt} is yours, it's pretty cool :-) Although if you don't mind, I would prefer not to include it for now. I'll keep it in mind for the future though!
  6. OK

And one more note. I'd prefer not to change the development version number yet. We're figuring out what should be included in the first version but until then it's pointless to change it IMO.

@pdrhlik pdrhlik added this to the v0.1.0 milestone Oct 4, 2018
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.

2 participants