natverse / rcatmaid

R package providing API access to the CATMAID web image annotation tool
https://natverse.github.io/rcatmaid
GNU General Public License v3.0
9 stars 6 forks source link

Updated for HTTP/2 calls fail in `catmaid` server #158 #159

Closed SridharJagannathan closed 4 years ago

SridharJagannathan commented 4 years ago

Updated for #158

jefferis commented 4 years ago

Thanks for this @SridharJagannathan. I guess you ran out of steam, when what was supposed to be a simple doc fix turned out to uncover several other problems. I think that most of these are related to changes in behaviour of R 4.0 (stringsAsFactors) / (https://github.com/r-lib/devtools/issues/2216) in which tests in \donttest blocks are now run by default!

SridharJagannathan commented 4 years ago

Thanks @jefferis I tried your suggestion of setting the env var but it still fails (infact in one of my previous commits i tried changing \donttest to \dontrun, which should do the same thing as the env var but it still failed), so the issue is not only due to the \donttest block but due to warnings being converted to errors by the texlive packages, the error message was a bit cryptic at that time so i decided to come back to it later (https://travis-ci.org/github/natverse/rcatmaid/jobs/696263033) you can see example here: (https://devops.stackexchange.com/questions/8014/travis-tests-for-r-package-fail-with-little-explanation). I have now changed that behaviour by using warnings_are_errors: false which lets the travis build pass, but I guess the larger problem is investigating the texlive issues.

jefferis commented 4 years ago

Hi @SridharJagannathan, I suspect that the remaining bug is not to do with texlive but actually in a different place, here:

https://travis-ci.org/github/natverse/rcatmaid/jobs/696269530#L968-L969

I haven't figured out the fix but it suggest some kind of code/documentation mismatch for this AV4b1 object.

SridharJagannathan commented 4 years ago

@jefferis The texlive issue seems to be present for other users too: https://travis-ci.community/t/build-failure-due-to-latex-font-warning-font-shape-t1-zi4-m-it-undefined-fixed-with/3238 which hasn't been yet resolved.

jefferis commented 4 years ago

Possibly but the issue with the AV4b1 is definitely a warning in R check (i.e. an error by default). We can sidestep the tex issue by just not building the PDF manual on travis. There is a travis yml setting for that. Like so, I think.

r_build_args: --no-manual 
r_check_args: --no-manual
jefferis commented 4 years ago

I think adding

#' @usage data(AV4b1) 

in catmaid.R should fix that other warning.

jefferis commented 4 years ago

OK. I think you've cracked it @SridharJagannathan! Thanks so much, G.

SridharJagannathan commented 4 years ago

Thanks @jefferis fixed the issue wrt AV4b1 and reverted to warnings_are_errors: true, the build passes now.

jefferis commented 4 years ago

Added a couple more example fixes. Now donttest tests work fine on my local machine using the l1em.virtualflybrain.org as target.