-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Visualization #212
base: develop
Are you sure you want to change the base?
Visualization #212
Conversation
Invokes Rscript pedigree_and_layout.R to compute the layout (depends on R kinship2 package), and bundles the output into an html template. The final output is an html file which contains all the visualization data except for some 3rd party libraries like JQuery, d3, and Bootstrap, which are currently being pulled from the web by the user's browser at runtime. Note: this functionality depends on pedigree_and_layout.R and the index.html template, which are not included in this commit, since they are part of the dng-vis repo. I'll probably add them in a separate commit.
Also checks that kinship2 and jsonlite R packages are installed and issues a warning if not.
Use find_package_handle_standard_args to get standard messages.
Can one of the admins verify this patch? |
I'm including |
Jenkins, okay to test
…On Dec 22, 2016 8:45 PM, "Anders Pitman" ***@***.***> wrote:
I'm including pedigree_and_layout.R and index.html, even though those are
part of the dng-vis repo and will change along with that. I'm not sure
how we want to handle that dependency, so I just did it like treecall where
we manually copy over a version for now.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#212 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGOHpr11SPosm-k1UNPQFBROQyzSnR_ks5rK0PwgaJpZM4LUjpp>
.
|
Current coverage is 58.64% (diff: 100%)@@ develop #212 diff @@
==========================================
Files 88 88
Lines 8382 7989 -393
Methods 520 520
Messages 0 0
Branches 4839 4779 -60
==========================================
- Hits 4832 4685 -147
+ Misses 1777 1685 -92
+ Partials 1773 1619 -154
|
How is it possible for coverage to have changed that much? |
Made the development and deployment pipelines much closer. The same scripts are now used for both for the most part. Also added argument parsing to dng mutmap
If mutmap is going to be an "official" part of DNG, then it will need to be developed as part of this repository. Also, why do we have a shell script that calls a Python script that calls an R script? Can't we just have an executable R script that does everything? Or am I missing something? |
Cool, I'll move all the visualization code to be under The multiple scripts are mostly to keep things modular. I don't want our layout to be tied to R, so I didn't write the other logic in that. I started in bash but switched to Python once I started added argument parsing. I originally had |
Right now, you are not doing anything with the shell and python script that you cannot do in R. Before we go to a three-language solution, I would like to see an attempt to try to do it with one script. |
I guess my biggest argument at this point is that it's already finished and working, and would be the same amount of work to change it later as it would be now. In general, anything that needs to be done in R will necessitate me first learning how to do that thing in R (argument parsing is a great example), whereas I'm already familiar with python and can add features quickly. I'd love to learn some more R but I don't think it's a good tool for this. I think the R dependency is the one we should be the most wary about. Eventually I would like to replace the layout calculation with C++ or javascript. Python is a much more common utility language, so other collaborators would expect to see it used for the sorts of things I'm doing here. If we do that logic in R, it will raise the bar for external contributions. |
Also getting rid of the shell shim would mean complicating the development pipeline. |
But I don't mind doing it if you sure that's the way you want to go. |
Another use case I just thought of. An entire build takes about 3 seconds currently. Most of that time is spent warming up R and loading the libraries. I was just working on a way to have python keep a warm Rscript instance running that you can just feed into it's stdin and get results back. This should cut the compile loop down to basically instantaneous, which would help development. This is all pretty straightforward in python, but I'm not sure if it's even possible in R. A quick search doesn't look too hopeful. |
I think this should be superceded by #213 |
No description provided.