Adds new end-points and improves to existing ones #5

sk-sahu · 2020-08-11T18:32:18Z

New API end-points

Create a Cohort
Extract Genotypic Data

Also improvements to existing ones with status check 200 for API calls and error message.

cgpu · 2020-08-13T10:05:41Z

.github/workflows/R-CMD-check.yaml

 name: R-CMD-check
+on: [push, pull_request]


check all the things sounds good to me!

cgpu · 2020-08-13T10:07:30Z

R/extract_genotypic_data.R

+#' extract_genotypic_data(baseurl= "https://cloudos.lifebit.ai",
+#'              auth = "Bearer ***token***",
+#'              teamid = "***teamid***")


can you make everythang snakecase eg
baseurl -> base_url
teamid -> team_id

in all occurrences?

R/extract_participants.R

cgpu · 2020-08-13T10:09:45Z

R/list_cohorts.R

@@ -29,15 +29,15 @@ list_cohorts <- function(baseurl,
                 query = list("teamId" = teamid,
                              "pageNumber" = page_number,
                              "pageSize" = page_size))
-  res <- httr::content(r)
-  if(length(res) == 0){
+  if (!r$status_code == 200) {
    message("No cohorts found. Or not able to connect with server.")


Here why message and not error to exit?

Good suggestion. There is stop() for error message and exit. I'll include that.

docs/404.html

cgpu

Minor request for snake_case 2 vars:

teamid
baseurl

…uild

sk-sahu · 2020-08-13T12:30:35Z

@cgpu I have tried to not build the vignettes --no-build-vignettes for R-CMD-Check as it require api-key. But this doesn't seems working. Any idea?

This is strange. Last time it worked in this - 168682f commit. Not sure how.

cgpu · 2020-08-18T14:28:48Z

This is strange. Last time it worked in this - 168682f commit. Not sure how.

@sk-sahu i don't see the line with the --no-build-vignettes in 168682f.

Can you point the relevant line by checking in Browse files?

sk-sahu · 2020-08-18T14:43:46Z

@sk-sahu i don't see the line with the --no-build-vignettes in 168682f.
Can you point the relevant line by checking in Browse files?

@cgpu That's the point (strange thing) it didn't have --no-build-vignettes in this 168682f commit. But somehow it didn't render. But from that point onward it started this error in next commit.

Then I added this -

cloudos/.github/workflows/R-CMD-check.yaml

Line 78 in 6bafcf7

    
           run: rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran", "--no-build-vignettes"), error_on = "warning", check_dir = "check")

cgpu · 2020-08-19T13:46:10Z

check/cloudos.Rcheck/cloudos-Ex.Rout

@@ -0,0 +1,147 @@
+
+R version 4.0.2 (2020-06-22) -- "Taking Off Again"


do we need this files under version control @sk-sahu ?

@cgpu No we don't.

This check directory created while local check and build. I need to delete this. Thank you for pointing out :)

I'll add this to .gitignore

cgpu · 2020-08-19T17:03:55Z

R/create_cohort.R

+                  encode = "json"
+  )
+  if (!r$status_code == 200) {
+    stop("Something went wrong. Not able to create a cohort")


nice, I see message replaced for stop

cgpu · 2020-08-19T17:06:30Z

R/create_cohort.R

+  )
+  if (!r$status_code == 200) {
+    stop("Something went wrong. Not able to create a cohort")
+  }else{


if (r$status_code == 200)

on this, I generally think if is more explicit and helps with matching assumptions with tests.

This is also Jenny Bryan approved, see here

That's a legit point 👍 This will make the code even more readable as well. Let me do that.

@sk-sahu thanks for being on the Jenny Bryan side of life 😄 !

One note, after checking the code:

cloudos/R/create_cohort.R

Lines 36 to 43 in c7d081e

)

if (!r$status_code == 200) {

stop("Something went wrong. Not able to create a cohort")

}

# parse the content

message("Cohort named ", cohort_name, " created successfully. Bellow are the details")

res <- httr::content(r)

# into a dataframe

Once you remove else, the idea is to replace it with the explicit if statement, not eliminate it altogether.
This is fine for now, I will open an issue and link it to your currently open PR to implement this in all functions.

So in principle as an example the following snippet, when if-ified would lokk like this:

cloudos/R/create_cohort.R

Lines 37 to 46 in 1e1ec00

if (!r$status_code == 200) {

stop("Something went wrong. Not able to create a cohort")

}else{

message("Cohort named ", cohort_name, " created successfully. Bellow are the details")

res <- httr::content(r)

# into a dataframe

res_df <- do.call(rbind, res)

colnames(res_df) <- "details"

return(res_df)

}

if (!r$status_code == 200) { stop("Something went wrong. Not able to create a cohort") } if (r$status_code == 200){ message("Cohort named ", cohort_name, " created successfully. Bellow are the details") res <- httr::content(r) # into a dataframe res_df <- do.call(rbind, res) colnames(res_df) <- "details" return(res_df) }

cgpu

lgtm! 👍

Only a minor note on (not) using else, see here #r473191206, but approving.

@cgpu

* adds new end-points and impv to previous ones * adds pkgdown html links * linting correction * R-cmd-check in every push and PR * minor correction to title * updates doc * chnages to snakecase for team_id and base_url * error message() -> stop() which will exit with message. * minor corrections * local build and check * --no-build-vignettes added for R-CMD-Check as it require api-key to build * Attempt to fix tests (respect --no-build) * Attempt to address #5 (comment) * syntax fix in YAML * vignette oved to README * cleaning * no else based on @cgpu 's review Co-authored-by: cgpu <38183826+cgpu@users.noreply.github.com>

adds new end-points and impv to previous ones

713210b

sk-sahu requested a review from cgpu August 11, 2020 18:32

sk-sahu added 5 commits August 12, 2020 00:13

adds pkgdown html links

a151daa

linting correction

c9d15ea

R-cmd-check in every push and PR

66f1c2b

minor correction to title

600b0f2

updates doc

168682f

cgpu reviewed Aug 13, 2020

View reviewed changes

R/extract_participants.R Outdated Show resolved Hide resolved

cgpu reviewed Aug 13, 2020

View reviewed changes

docs/404.html Show resolved Hide resolved

github-pages bot temporarily deployed to github-pages August 13, 2020 10:13 Inactive

cgpu requested changes Aug 13, 2020

View reviewed changes

sk-sahu added 4 commits August 13, 2020 16:15

chnages to snakecase for team_id and base_url

9ae5782

error message() -> stop() which will exit with message.

753055a

minor corrections

01bd553

local build and check

1667fee

github-pages bot temporarily deployed to github-pages August 13, 2020 11:55 Inactive

--no-build-vignettes added for R-CMD-Check as it require api-key to b…

a9476a9

…uild

github-pages bot temporarily deployed to github-pages August 13, 2020 12:04 Inactive

Attempt to fix tests (respect --no-build)

6bafcf7

github-pages bot temporarily deployed to github-pages August 18, 2020 14:44 Inactive

Attempt to address #5 (comment)

55b6ebb

github-pages bot temporarily deployed to github-pages August 18, 2020 14:54 Inactive

syntax fix in YAML

4e883e3

github-pages bot temporarily deployed to github-pages August 18, 2020 18:44 Inactive

vignette oved to README

78b494f

github-pages bot temporarily deployed to github-pages August 19, 2020 13:31 Inactive

sk-sahu requested a review from cgpu August 19, 2020 13:35

cgpu reviewed Aug 19, 2020

View reviewed changes

cleaning

1e1ec00

github-pages bot temporarily deployed to github-pages August 19, 2020 13:57 Inactive

sk-sahu requested a review from cgpu August 19, 2020 14:14

cgpu reviewed Aug 19, 2020

View reviewed changes

cgpu self-requested a review August 19, 2020 17:12

cgpu approved these changes Aug 19, 2020

View reviewed changes

no else based on @cgpu 's review

5b218e1

sk-sahu merged commit c7d081e into devel Aug 19, 2020

sk-sahu deleted the new-api branch August 19, 2020 18:34

cgpu mentioned this pull request Aug 20, 2020

Replace else with explicit if statement in R functions #7

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds new end-points and improves to existing ones #5

Adds new end-points and improves to existing ones #5

sk-sahu commented Aug 11, 2020

cgpu Aug 13, 2020

cgpu Aug 13, 2020

sk-sahu Aug 13, 2020

cgpu Aug 13, 2020

sk-sahu Aug 13, 2020

cgpu left a comment

sk-sahu commented Aug 13, 2020

cgpu commented Aug 18, 2020

sk-sahu commented Aug 18, 2020 •

edited

Loading

cgpu Aug 19, 2020

sk-sahu Aug 19, 2020

sk-sahu Aug 19, 2020

cgpu Aug 19, 2020

cgpu Aug 19, 2020 •

edited

Loading

sk-sahu Aug 19, 2020

cgpu Aug 20, 2020

cgpu left a comment

		@@ -0,0 +1,147 @@

		R version 4.0.2 (2020-06-22) -- "Taking Off Again"

	)
	if (!r$status_code == 200) {
	stop("Something went wrong. Not able to create a cohort")
	}
	# parse the content
	message("Cohort named ", cohort_name, " created successfully. Bellow are the details")
	res <- httr::content(r)
	# into a dataframe

	if (!r$status_code == 200) {
	stop("Something went wrong. Not able to create a cohort")
	}else{
	message("Cohort named ", cohort_name, " created successfully. Bellow are the details")
	res <- httr::content(r)
	# into a dataframe
	res_df <- do.call(rbind, res)
	colnames(res_df) <- "details"
	return(res_df)
	}

Adds new end-points and improves to existing ones #5

Adds new end-points and improves to existing ones #5

Conversation

sk-sahu commented Aug 11, 2020

New API end-points

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgpu left a comment

Choose a reason for hiding this comment

sk-sahu commented Aug 13, 2020

cgpu commented Aug 18, 2020

sk-sahu commented Aug 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgpu Aug 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgpu left a comment

Choose a reason for hiding this comment

sk-sahu commented Aug 18, 2020 •

edited

Loading

cgpu Aug 19, 2020 •

edited

Loading