cip-riu / brapi Goto Github PK

Goal: to only add deprecation information to the outdated function. Have a way to automatically extract that information and compile it into a user visible list.

For an option see: https://www.r-bloggers.com/r-deprecate-functions-with-roxygen2/

Created the 'deprecate' branch to work on it. Implemented for two functions. However, it appears roxygen2 somehow inserts non-ASCII code in the generaed Rd files for the functions on my Windows machines, which then causes warnings in the R CMD check.

Stopped for the moment. Until further investigation.

Hi Reinhard,

Please close this issue, I figured it out. However, the part I commented out in issue 8, is it needed in brapiGET.R. What is your intention for this part?

The out object created in the if statement is not being used inside the function, and appears to be obsolete

Hi Reinhard,

Perhaps you noticed it already, but I am reviewing your written functions. I have noticed that you most of the time match arguments by position. I would propose to write function arguments in full and match them by name and position. This again to improve readability of the code and to understand what arguments are being passed when using them inside other functions.

Another observation is that you have your indentation set to 4 spaces. I propose to reduce it to 2 spaces, a setting commonly advised in books about R package development (e.g. http://r-pkgs.had.co.nz/r.html section Indentation).

Let me know what you think of these changes.

Regards,

The reason I propose this, is that the url of the call is not linked to studies. It is a separate call /brapi/v1/seasons.

Do you concur? It would require a few renaming actions.

Hi Reinhard,

The trl2tbl.R Function is not working properly. Neither on the BTI bases (e.g. sweetpotatobase) nor on BMS. I assume that the function name implies to table the trials. Could you let me know whether this a correct assumption? I will try to modify it to do what it is supposed to do.

Regards,
Maikel

after the addition of:

• datatypes <- NULL
• methods <- NULL
• versions <- NULL
  the ba_calls function is not working properly anymore. These object serve no function in the try({}).
  @c5sire Could you explain why you added these?

Hi Reinhard,

Is there a reason why you define the brapi::can_internet function? The curl::has_internet does exactly the same.

Regards,

Following up

Forget about the question above, I suddenly realized that your can_internet function can check whether internet connection exists based on any url whereas has_internet has an empty argument.

I was a bit to fast posting this issue.

Latest changes

study 2 table function (std2tbl.R) needs to be adapted for additionalInfo columns. Now they are given as additionalInfo.. I think it would be better to remove "additionInfo." from the column name, as done in trl2tbl2.R

So far only the GET method for ba_studies_layout has been implemented. To be compliant with v1.2 the PUT method needs to be added.

Hi Reinhard,

Could you check and correct the README.Rmd file in the CIP-RIU repository? There are many links, that still refer to c5sire/brapi and also a read.csv function referring to apps/brapi/data/calls.csv that does not seem to exist.

Regards,
Maikel

Hi Reinhard,

In my fork of your brapi I have changed the brapi_con call by taking out the brapi_auth call. In my opinion the brapi_con should only be used to con(struct) the brapi S3 construct. Next I use this construct in brapi_auth to authenticate a user. I have also modified the calls in such a way that they can be used for the Breeding Management System of IPB. In the zzz.R file I have added a lot of crops that can be used in the BMS.

Have a look and let me know what your thoughts are on these changes. If you like them I could make a Pull request and merge the changes into your brapi on c5sire.

Regards,

Maikel

In the pipe to generate to out object from the input res object the following function tidyjson::gather_array appears twice without parentheses and, therefore, also without function arguments. Could you check if this is correct and functioning as you expect?

I commented out the part below # Get JSON, because it only checks whether format == "json" but further does nothing

as mentioned in issue #9 the out object in the if statement is not used. Please check this.

deprecate crops

Hola Reinhard, que tal, espero que todo muy bien,

Una pregunta, queremos conectarnos a Sweepotato Base a través de BraPi, hemos leido tu tutorial y dice lo siguiente:

# Optionally: authenticate yourself
con = ba_connect(user = "user", password = "password") %>% ba_login()
ba_crops(con) # which crops are available
ba_calls(con) # which calls are available

Deseamos saber:

• Como obtenemos el user y el password para logearnos usando la función ba_connect en R?
• Con que función de brapi podemos obtener (get) las tablas de los libros de campo (fieldbooks) ? Estas tablas serian parecidas a la hoja FieldBook de HiDAP.

Gracias de antemano
Saludos cordiales.

Ivan P.

Hi Reinhard,

Could you explain what the purpose of brapi::set_brapi is? To me it seems a bit a silly function, since whatever you put in it always sets the global brapi$db to the url you put in and brapi$port to the value you put in or when not provided brapi$port will default to 3000.

Regards,

Maikel

@omarbenites favor reconectar appveyor a este repo.

Hi Reinhard,

I have been trying to use your implementation of "Study Observation Units as a Table" (your studies_table.R function). It does not return the correct table when there are missing values in the JSON (represented as null). I have solved this problem by using the fromJSON function from the jsonlite package. Here is a short example:

• resp <- httr::GET(...), where you obtain the resp(onse) from your GET call
• jsonlite::fromJSON(httr::content(resp, as = "text")), parse the resp(onse) into a list, where the table is given in $result$data
• the column headers are given in the list by $result$headerRow concatenated with $result$observationVariableNames

Hope this information will be useful for you as well,

Regards,
Maikel

naming pattern for PUT calls: _update

@c5sire I tested and updated the ba_locations call. However, when checking the package it produces an error, which will probably also break the CI.

I guess it involves the tests you implemented. Could you check this? The ba_locations call now works on BMS, sweetpotatobase and the test-server!
regards,
Maikel

Hi Reinhardt!
The brapi R package is great.
I was looking at the ba_studies_table() function and was wondering, if you plan to expand to collect multiple studyDbId at once?
Thanks for your feedback

Guillaume

@c5sire

=> When installing:
devtools::install_github("CIP-RIU/brapi")

=> an error message would be thrown:
ERROR: dependency ‘tidyjson’ is not available for package ‘brapi’

• removing ‘/Library/Frameworks/R.framework/Versions/3.4/Resources/library/brapi’
  Installation failed: Command failed (1)

=> tidyjson was apparently removed from CRAN.

=>You can get around this by adding the new git repo:
devtools::install_github(“sailthru/tidyjson”)

Will improve testability and performance of re-formatting.
Also more transparent

Better explanation of steps and parameters.

Hi,

When running this code

sp_base <- ba_db()$snpseek
ba_programs(sp_base, rclass = "data.frame")

I'm getting this error

Error : lexical error: invalid char in json text.
<tit
(right here) ------^
Thanks.

The white list of known databases contains the following
snpseek <- as.ba_db(secure = FALSE,
protocol = "http://",
db = "snp-seek.irri.org",
port = 80,
apipath = "ws",
user = "snpseek-user",
password = "snpseek-user-pass",
multicrop = FALSE,
crop = "rice",
token = "",
bms = TRUE)
Is the last argument assignment TRUE? I don't think this database is running BMS, otherwise multicrop should probably also be TRUE.

As you can see I have restructured the ba_db.R file to match the function argument order as given in as.ba_db.R and put each argument on a new line for readability.

Hola Reinhard, hoy tuvimos unos problemas usando el R-BRAPI. Debajo el error:

library(brapi)
library(magrittr)
white_list <- ba_db()
sp_base <- ba_db()$sweetpotatobase
dt = ba_studies_table(sp_base, 
                      studyDbId = "162")

Error:

Dale una revisada al problema, dado que estamos haciendo un training. Te lo agradeceríamos bastante.

Saludos

Just noticed that the last build on appveyor was 6 months ago. There seems to be something wrong there.

Always return all standard fields in data.frame too even if all have no data.

I still believe for readability it is better.

Here is a discussion: http://stackoverflow.com/questions/11738823/explicitly-calling-return-in-a-function-or-not

Let's both read it and decide our standard. I'll be in touch.

Today I have done some code repairs and made the object coming from brapi_db() the same as from connect(). Apart from that I did some styling. I must say, you have done great work !!

Regards,

Maikel

Just noticed that ba_studies_germplasm_details has problems with pagenation, when trying it on the test-server.
con <- ba_db()$testserver
When asking for pageSize = 1, the output is unexpected. sgp2tbl.R needs modification in nrow() line

Hi Reinhard,

In the brapiGET function the part:

tryCatch({...
}, error = function(e) stop(paste0(e, "\n\nMalformed request.")))

is causing error messages that need not to be generated. For example when the lst object has the value NULL. Is this part really need in this function? When I comment it out, all works fine with any problems.There is also still a TODO remark, which makes me question the reliability of the code.

Could you look into it or comment on what I just mentioned.
Regards,
Maikel

The try({}) contains the following:
body <- list(observationVariableDbIds = observationVariableDbIds %>% as.list(),
ontologyXrefs = ontologyXrefs,
ontologyXrefs = ontologyXrefs,
ontologyXrefs = ontologyXrefs,
scaleDbIds = scaleDbIds,
names = names,
datatypes = datatypes,
traitClasses = traitClasses)
Are the multiple instances for ontologyXrefs correct ?

Hi there,

I'm a cassava base user. Most of my analysis is done in R. I'm trying to integrate direct DB calls into my pipeline using your package. I was wondering what you recommended or planned for downloading multiple trials?

Below is my solution, which works but is VERY VERY slow (hours or days) and often times out... in contrast direct calls to download the same data using the GUI / web interface for the DB takes minutes/hours.

library(tidyverse); library(brapi)
db<-ba_db()$cassavabase
trials_1<-db %>% 
    ba_studies_search(programDbId = "162") %>% 
    mutate(seasons=as.numeric(seasons)) %>% 
    filter(seasons >= 2015,
           seasons <= 2016) %>% 
    mutate(studyDbId=as.character(studyDbId),
           data=map(studyDbId,~ba_studies_table(db,.)))

Hi guys @c5sire @mverouden
I was running this R-BRAPI and now this show an error that not appear in the past. Below the code and errors.

library(brapi)
library(dplyr)

white_list <- brapi::ba_db()
sp_base_credentials <- white_list$sweetpotatobase
trial_table <- brapi::ba_trials(con = sp_base_credentials)

Omar

The list study details call (/studies/{studyDbId}) works for simple cases, e.g.
con <- ba_db()$sweetpotatobase; ba_studies_details(con = con, studyDbId = "182)

When the case gets more complicated, the output is horrible, e.g.
con <- ba_db()$testserver; ba_studies_details(con = con, studyDbId = "1001")

The problem is caused by the stdd2tbl() function, which is in serious need of revision. Suggested strategy would be to use a similar approach as in the trld2tbl2() function.