Changes in read_data() #65
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ation of the data and import data tha has been renamed. Also, adds feedback to the user about the use of "root_path" , "file" or none of them to find the data files. Also, remove "dic_list" argument, and put "dataset" instead, the function now get the dictionary with the wrapper get_import_dictionary() Also, turn 'metadata' parameter into optional, if is not inputed the function will get the metadata with read_metadata(dataset)
|
Ok. I like the read_data() idea a lot. The other one adds a bit of complexity, so I don't know. Lets decide on monday |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@lucasmation This request makes significant changes in both main_import_functions.R and import_wrapper_functions.R, so do not accept without reviewing.
I propose some modifications in our main functions to improve the user experience in 2 points:
Adds the possibility to point direct to a file in the import functions
To do that i added a new argument
fileto the functions. The new call becames:d<- read_PME(ft = "vinculos",i = "2012.01", file= "C:/.../PME/PMEnova_2015/PMEnova.012012.txt")The problem i see is confusion between
root_pathandfile. To partially solve it i made the function to give feedback for the user linking the filled parameters with the expected behavior.Example:
d<- read_PME(ft = "vinculos",i = "2012.01", file= "C:/.../PME/PMEnova_2015/PMEnova.012012.txt")Simplify read_data() to turn feasible to the user to use it instead of the wrapper functions
To do that, i made the argument
metadataoptional and excluded the argumentdic_listand added an argumentdataset, that should receive the name of the dataset to be imported.The old call to
read_datawas:data<-read_data(ft = "pessoas", i = 2014, read_metadata("PNAD"), dic = PNAD_dics,root_path = root_path)Where PNAD_dics was an object with all dictionaries and a confusing construction. Although it is easy to obtain it with
data(PNAD_dics).The new call is:
data<-read_data(dataset = "PNAD", ft = "pessoas", i = 2014, root_path = root_path, file = file)Where every argument has a unique and easy meaning.
As we were previously calling
data("PNAD_dics")inside the wrapper function, this also solves some warnings indevtools:check()about the use of global variables.