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

[Bug] Can't unset repoDir parameter in aoptions #176

Closed
MarcinKosinski opened this issue Jan 7, 2016 · 2 comments
Closed

[Bug] Can't unset repoDir parameter in aoptions #176

MarcinKosinski opened this issue Jan 7, 2016 · 2 comments

Comments

@MarcinKosinski
Copy link
Collaborator

If one would like to unset the repoDir parameter to be NULL as it is from the start (when the archivist package is loaded), it is impossible because when the second argument of aoptions is set to NULL then this function returns the argument instead of setting this parameter.

> library(archivist)
Welcome to archivist (version: 1.9.3.0).
> aoptions('repoDir')
NULL
> aoptions('repoDir', 'example')
[1] "example"
> aoptions('repoDir')
[1] "example"
> aoptions('repoDir', NULL)
[1] "example"

This functionality is crucial to unset repoDir when one would like to work with GitHub repo after he set Local repo and is using aread function that uses loadFromRepo function that checks global parameters. I recommend to add forceNULL parameter to the aoptions function. This is the cause of #175

MarcinKosinski added a commit that referenced this issue Jan 7, 2016
@MarcinKosinski
Copy link
Collaborator Author

@pbiecek what do you think about such solution?

@MarcinKosinski MarcinKosinski changed the title Can't unset repoDir parameter in aoptions [Bug] Can't unset repoDir parameter in aoptions Jan 10, 2016
@pbiecek
Copy link
Owner

pbiecek commented Jan 11, 2016

Will be better to find some other solution. Here the forceNULL is only to cover one special case. And NULL is not a real value it's lack of value.

Maybe we can use the value NA?
aoptions('repoDir', NA)
will work as expected and loadFromRepo can be prepared how to deal with NA.

Or use name like unset instead of forceNULL?

The second option is better since the name 'unset' corresponds to the expected behaviour of the function.

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

No branches or pull requests

2 participants