-
Notifications
You must be signed in to change notification settings - Fork 40
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
PR - Include Source generation script/Change domain #25
Conversation
Is this PR ready to review? Looks like it still stores all the rst files. |
My bad. Forgot to remove the doc folder. Pls see latest commit |
Looks great! One comment: Travis seems to take >30 minutes and is timing out, whereas in #20 it only took 12 minutes. In principle, the only change is the download server. Is this slowdown really because of the download? |
I noticed that. I think because Travis and Github are well integrated, it is fast. The flip side is that I was testing it locally when I was @ Columbia and it took < 200 secs and all tests passed. I am ssuming Travis has some restrictions on downloading data from external servers. |
Got it. This sounds like a pervasive issue which is relevant to this PR but is more about #23 and how to handle Travis downloading many data sets. I propose merging the PR as is and leaving that issue open. Thoughts? |
Agree. For review purposes, Would request you to clone the repo and run tests locally and verify. It works at my end, but need you to validate it.. |
Done! |
observations.r
and observation.tests.rgeneration scripts in
observations/src`csv
file and it ignored thedatasets.csv
file which was required for generation