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

PR2: Fix spelling errors and add things to dictionary.txt #257

Merged
merged 9 commits into from
Oct 2, 2020

Conversation

cansavvy
Copy link
Contributor

@cansavvy cansavvy commented Oct 1, 2020

Purpose:

The second PR mentioned on #239 where I fix all the spelling errors and add the words to the dictionary.txt that we need.

As a side consequence, softwares (#244) and ID (#243) and Rmd -> .Rmd inconsistencies have been resolved and added to the CONTRIBUTING.md if they were not previously there.

Closes #243
Closes #244
Closes #239

Strategy

A lot of iterations of running spell check, searching in the project for where those words occurred and determing whether they were legit words and should be added to the dictionary.txt or if they were legit errors and in that case I fixed them.

For non-updated notebooks that had manual citations eg. Yaari et al, _PLoS_ I deleted those because they will be done automatically, but I kept the paper links so that when we do get to updating that notebook we know what the paper and its reference should be.

Unfortunately spelling::spell_check_files() isn't great with hyphens so you will note I put fake words like vivo even though I really wanted it to put in-vivo but it would not take that.

For reviewers:

I wouldn't worry about examples that have not been updated yet. I've basically made them so they will pass but they will be further refined when we get to those examples. I would mostly take a look at dictionary.txt and CONTRIBUTING.md and see if that makes sense, as well as the spelling fixes you will see sprinkled throughout.

Docker/Snakemake rendering components

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

I made a number of suggestions to the spelling dictionary, which are best addressed individually below.

I also have a couple of higher level questions about how the spellcheck actually works: do we need plurals in cases of words like "subtype", or does adding that give us "subtypes" for free (if I wrote a spellchecker...I am not sure which decision I would make). Similarly, possessives: if we have "dataset", do we need "dataset's"?

components/dictionary.txt Outdated Show resolved Hide resolved
components/dictionary.txt Show resolved Hide resolved
components/dictionary.txt Outdated Show resolved Hide resolved
components/dictionary.txt Outdated Show resolved Hide resolved
components/dictionary.txt Outdated Show resolved Hide resolved
components/dictionary.txt Outdated Show resolved Hide resolved
components/dictionary.txt Show resolved Hide resolved
components/dictionary.txt Show resolved Hide resolved
components/dictionary.txt Show resolved Hide resolved
components/dictionary.txt Outdated Show resolved Hide resolved
@cansavvy
Copy link
Contributor Author

cansavvy commented Oct 1, 2020

Shouldn't this be a proper reference with @Tag (or is this a not-yet-updates page and we don't need to worry about it?)

This module has not been updated yet. (#215) My changes to not yet updated modules are basically just to make the spell check happy for now.

@cansavvy cansavvy marked this pull request as ready for review October 1, 2020 16:47
@cansavvy cansavvy requested a review from cbethell October 1, 2020 17:05
Copy link
Contributor

@cbethell cbethell left a comment

Choose a reason for hiding this comment

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

Looks good and everything checks out! Just one question re running the spell check script locally (it worked fine once I got the package installed, see below for more details!).

CONTRIBUTING.md Show resolved Hide resolved
@cansavvy cansavvy requested a review from cbethell October 2, 2020 11:51
Copy link
Contributor

@cbethell cbethell left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@cansavvy cansavvy merged commit 9bc1ee6 into master Oct 2, 2020
@cansavvy cansavvy deleted the cansavvy/sp-check-2 branch October 2, 2020 13:26
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.

Template fix: softwares -> software Style add/update: IDs NOT ids or Ids Automation: Spell checking files
4 participants