bomeara commented 5 years ago

There's now caching. Could make the default tempdir rather than null, so for repeated plotting of similar organisms is fast.

pnovack-gottshall commented 5 years ago

Great to see this being added. Be aware that CRAN has gotten pickier when a function changes the working directory. A simple fix is to do the following (in sequential lines):

odir <- setwd(tempdir())
on.exit(setwd(odir))

It worked fine in my ecospace::calc_metrics() function. Another benefit is that it sets CPU-specific working directories, which is convenient if using parallel computing across different cores/CPUs.

dwbapst commented 5 years ago

Having not looked at the code at all, and thus being 0% informed on what godless things you've done to my beautiful code ( ;p ), I don't understand why we would use setwd() at all yet - caching should be done in a hidden directory, by default something like /.cached.phylopic.paleotree in the working directory, but which can be then altered top-level by the user with arguments.

Anyway just saying this here for now so I remember what my immediate response was later, after I've looked at these changes.

dwbapst commented 5 years ago

Also @pnovack-gottshall have you seen yet what we've been doing all this for? Check out this tweet from @jwiggi18

https://twitter.com/jwiggi21/status/1095006192873676801

pnovack-gottshall commented 5 years ago

Nope, no idea :) My spidey-sense just goes up when I hear R packages calling setwd(). Will check out!

Phil

On 2/13/2019 1:22 PM, David Bapst wrote:

Also @pnovack-gottshallhttps://github.com/pnovack-gottshall have you seen yet what we've been doing all this for? Check out this tweet from @jwiggi18https://github.com/jwiggi18

https://twitter.com/jwiggi21/status/1095006192873676801

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/dwbapst/paleotree/pull/14#issuecomment-463330453, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AOfpfz1O1BlX3o99yc9nMa3-ZWYNTfJzks5vNGUbgaJpZM4awzx5.

--

  Phil Novack-Gottshall, PhD
  Professor                            pnovack-gottshall@ben.edu<mailto:pnovack-gottshall@ben.edu>
  Department of Biological Sciences
  Benedictine University
  5700 College Road
  Lisle, IL 60532

  Office: 332 Birck Hall
  Lab: 316 Birck Hall
  Phone: 630-829-6514
  Fax: 630-829-6547
  https://pnovack-gottshall.wixsite.com/home

  Spring 2019 office hours:  Tues/Thurs 9:30-11:00 AM
                             Wed 10 AM - 12:15 PM

  If you have urgent academic advising questions, please contact
  Anne Baysinger (Birck 130)

pnovack-gottshall commented 5 years ago

Ooh, how do I get this interface on PBDB? Is it the development version? Must know more!!

Phil

On 2/13/2019 1:22 PM, David Bapst wrote:

Also @pnovack-gottshallhttps://github.com/pnovack-gottshall have you seen yet what we've been doing all this for? Check out this tweet from @jwiggi18https://github.com/jwiggi18

https://twitter.com/jwiggi21/status/1095006192873676801

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/dwbapst/paleotree/pull/14#issuecomment-463330453, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AOfpfz1O1BlX3o99yc9nMa3-ZWYNTfJzks5vNGUbgaJpZM4awzx5.

--

  Phil Novack-Gottshall, PhD
  Professor                            pnovack-gottshall@ben.edu<mailto:pnovack-gottshall@ben.edu>
  Department of Biological Sciences
  Benedictine University
  5700 College Road
  Lisle, IL 60532

  Office: 332 Birck Hall
  Lab: 316 Birck Hall
  Phone: 630-829-6514
  Fax: 630-829-6547
  https://pnovack-gottshall.wixsite.com/home

  Spring 2019 office hours:  Tues/Thurs 9:30-11:00 AM
                             Wed 10 AM - 12:15 PM

  If you have urgent academic advising questions, please contact
  Anne Baysinger (Birck 130)

bomeara commented 5 years ago

Thanks, @pnovack-gottshall. We'll fix the setwd, and thanks for the info -- keeping up with CRAN's standards is always a job. We're using @dwbapst's paleotree to help make trees for https://github.com/jwiggi18/HistoryOfEarth -- it's still very active in development (it might work one minute, be down the next) but we expected it to be stable by the end of next week.

bomeara commented 5 years ago

Actually, looking at the new code -- we do have a cacheDir, but we don't ever call setwd() in this code. Did I miss something?

codecov-io commented 5 years ago

Codecov Report

Merging #14 into developmentBranch will decrease coverage by <.01%. The diff coverage is 0%.

@@                 Coverage Diff                  @@
##           developmentBranch     #14      +/-   ##
====================================================
- Coverage               1.32%   1.32%   -0.01%     
====================================================
  Files                     73      73              
  Lines                   8078    8099      +21     
====================================================
  Hits                     107     107              
- Misses                  7971    7992      +21

Impacted Files	Coverage Δ
R/plotPhylopicTreePBDB.R	`0% <0%> (ø)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 312301f...db33f46. Read the comment docs.

dwbapst commented 5 years ago

I think all functionality here has now been subsumed by the in-development plotting branch

dwbapst / paleotree

Caching #14

Codecov Report