Update docs with minor erro

[nzgwynn]

I found a minor error and corrected it!

main...nzgwynn:ipumsr:patch-1

[robe2037]

Thanks for the contribution. The original value is actually correct—by default, if no save argument is provided, set_ipums_api_key() uses the value of the overwrite argument to determine whether to save the key or not. This is because there is no way to overwrite a key in .Renviron if it isn't being saved in the first place (for instance, in a case like the following we force save = TRUE: set_ipums_api_key("key", overwrite = TRUE)). This is mostly for convenience.

[nzgwynn]

Thank you!

When I ran the following coding, this was the result:

> set_ipums_api_key(IPUMS_API_KEY, save = overwrite, overwrite = FALSE, unset = FALSE)
Error:
! object 'overwrite' not found

I updated to this:

> set_ipums_api_key(IPUMS_API_KEY, save = TRUE, overwrite = FALSE, unset = FALSE)
The environment variable IPUMS_API_KEY has been set and saved for future sessions.

and the function ran and gave me the expected result. Because you say the proposed suggestion is not a fix, I am unsure how others can avoid this problem, especially beginners. Thanks in advance for any support you can provide, and have a good day!

[robe2037]

Ah, this is because save = overwrite is only intended to set the default value of save, not to be provided directly by the end user.

In your first line of code, you request save = overwrite explicitly in the function call, so R looks for a defined variable called overwrite in your environment, which it doesn't find. In contrast, if no value of save is provided by the user, set_ipums_api_key() only looks for the default value overwrite once it is within the function environment, where it does exist since it is itself an argument to the function (this is because of lazy evaluation).

Note that the function reference documentation is intended to show the available arguments and default values, not necessarily to be copied directly into R, so use caution when doing so.

So, what you're probably after is actually:

set_ipums_api_key("insert-key-here", save = TRUE)

or, if you only want to set your key for the current session and not save it for future sessions:

set_ipums_api_key("insert-key-here")

Note that the omitted arguments (overwrite, unset, etc.) are being used with the default values that are shown in the function documentation, so

set_ipums_api_key("insert-key-here", save = TRUE)

is equivalent to

set_ipums_api_key("insert-key-here", save = TRUE, overwrite = FALSE, unset = FALSE)

Hope this helps clarify things. If you're new to R function composition, Chapter 6 of Advanced R is a great place to start.

Note as well the example included in the API vignette on the ipumsr website.

[nzgwynn]

I am aware of why it does this, though a beginner would not be. I copied it as there is no example coding provided, you might consider adding the example from your article to the documentation.

[robe2037]

I've made a few tweaks to the documentation in an attempt to make this more clear.

@dtburk I'll leave it up to you whether you want to merge this in!

[dtburk]

Thanks for taking the time to file an issue @nzgwynn, and thanks for addressing this confusion @robe2037!

I personally prefer the existing save = overwrite, overwrite = FALSE pattern, but I can see how that pattern might be confusing to users who haven't encountered it before.

I don't really like NULL as the default of save, since save is a logical argument, but I see how it addresses @nzgwynn's point that the function usage statement couldn't be run as-is.

What if we just set the default values of both save and overwrite to be FALSE, but if the user sets overwrite to be TRUE, we assume that save should also be TRUE? We could note this behavior in the argument documentation for save. This would be less explicit than the save = overwrite, overwrite = FALSE pattern, but might also be less likely to cause confusion. What do you both think?

[robe2037]

I'd probably lean toward keeping save = overwrite over setting save = FALSE. In general I don't love modifying arguments internally because it opens the door for confusion if arguments are silently modified without the user realizing. NULL at least is the standard way to signal that a default may be set internally, but I agree that it doesn't communicate as clearly that the default in most cases will be FALSE.

I still think save = overwrite is the most succinct way to communicate exactly what happens to the argument in different scenarios. Ultimately this is a really minor use-case, though, so I don't have a strong preference. If we want to go with that we can just add the examples to the docs and leave it at that.

Also, I'm not totally sure where the CI failures are coming from, but it appears to be R 3.6 related. I'm confident it's not coming from these tiny changes...

[dtburk]

I just added a commit (aafffbc) that reverts the default value of save to overwrite and adds two more usage examples. @nzgwynn do you think the added usage examples could be enough to minimize confusion among users not familiar with the practice of using one argument as the default value of another argument?