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

Add argument to **unname_all_chunks** specifying which chunks to unname #22

Merged
merged 9 commits into from
Apr 4, 2019
Merged

Conversation

HanOostdijk
Copy link
Contributor

@HanOostdijk HanOostdijk commented Mar 29, 2019

The current version of unname_all_chunks removes all chunk labels except setup.

My proposal is to add an argument chunk_name_prefix (chunk name prefix) that when used will indicate which labels will be unnamed by specifying the prefix of these labels.
The default value for this argument is NULL and in that case the current processing will be done: all labels removed except setup.
For the two cases an example is given in the documentation of the function unname_all_chunks.

Use case: I sometimes refer to existing labels by using the chunk option ref.label . For an example of ref.label see e.g. Chunk Reference/Macro . After using name_chunks I have a set with 'own' labels and a set with 'generated' labels. When I later decide that I want to insert new chunks, I would prefer to unname only the 'generated' labels and then apply name_chunks again. With the suggested change, this can be done by specifying as chunk_name_prefix 'the filename with extension stripped' followed with a '-' (dash) that is used in name_chunks.

Apart from unname_all_chunks I also inserted a second named chunk in example4.Rmd in the example folder.

I am not very familiar with the PR process. Let me know when I made mistakes.

Edited:

  • use case inserted
  • ch_n_p argument renamed to chunk_name_prefix following suggestion by reviewer
  • section about package stringr is removed because the stringr functions are no longer used

unname_all_chunks now accepts argument `ch_n_p` with the prefix of the chunknames to be unnamed
@coveralls
Copy link

coveralls commented Mar 29, 2019

Pull Request Test Coverage Report for Build 74

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 97.087%

Totals Coverage Status
Change from base Build 68: 0.2%
Covered Lines: 100
Relevant Lines: 103

💛 - Coveralls

@HanOostdijk
Copy link
Contributor Author

Following up on the coverage test I see that I will have to add to
.\namer\tests\testthat\test-unname_all_chunks.R
the case that the ch_n_p argument is not NULL.

added test for case is.null(ch_n_p) == FALSE
changed test for case is.null(ch_n_p) == TRUE
@maelle
Copy link
Contributor

maelle commented Apr 1, 2019

👋 @HanOostdijk, thanks for your PR! It is a generally welcome feature, I'll comment about details.

DESCRIPTION Outdated Show resolved Hide resolved
DESCRIPTION Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
@maelle
Copy link
Contributor

maelle commented Apr 1, 2019

I'd prefer the name of the argument to be longer and therefore more explicit e.g. chunk_name_prefix. By the way, could you describe your use case for that?

R/unname_all_chunks.R Outdated Show resolved Hide resolved
R/unname_all_chunks.R Outdated Show resolved Hide resolved
HanOostdijk and others added 5 commits April 2, 2019 12:25
implemented the changes suggested by reviewer
unname_all_chunks: made sure 'setup' is not unnamed
testthat/test-unname_all_chunks: added test for this
@maelle
Copy link
Contributor

maelle commented Apr 4, 2019

@HanOostdijk I've pushed minimal changes to your branch and will merge if you agree with my adding you as contributor to DESCRIPTION. I did that via desc::desc_add_author_gh("HanOostdijk") so hopefully the info is correct.

@maelle
Copy link
Contributor

maelle commented Apr 4, 2019

One last thing actually, could you please add a paragraph about your use case in the vignette and README after "There's also name_chunks for use on a single R Markdown file; and unname_all_chunks to unname all chunks of a single R Markdown file which can be useful when cleaning your chunk labels."? Thanks!

DESCRIPTION: added ORCID
README.md: added use case unname_all_chunks
vignette: added use case and example
role = "ctb"))
role = "ctb",
comment = structure("0000-0001-6710-4566", .Names = "ORCID"))
)
Description: It names the 'R Markdown' chunks of files based on the filename.
Copy link
Contributor

Choose a reason for hiding this comment

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

yay ORCID 🎉

@maelle maelle merged commit 731fae9 into jumpingrivers:master Apr 4, 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.

3 participants