Closed chilampoon closed 4 years ago
Looks good! My only suggestion is it might be good to store the stable id Strings used in this PR in named variables that describe what the Reactome instance is they are referring to.
I was intended to do that in the vignette of ContentService package, but some names are just too long...
@chilampoon I've managed to get through everything except for the vignette. I'm going over it briefly, but the HTML is too large to render in GitHub, so I'll need to find another solution for commenting on it. I'll get to that tomorrow and review it in more detail! Maybe I'll try copying the text into a Google Doc that I can share with you.
Or perhaps commenting on the rmarkdown file vignettes/Introduction.Rmd
?
I found the vignette to be of great quality, even learning a few things I didn't know! The diagram visualizations are great as well. 👍 👍 👍 I will approve but please make the requested changes in the vignette before merging
graph$node$properties