avoid warning when missing R cache dir

r-lib / styler

Non-invasive pretty printing of R code

https://styler.r-lib.org

Other

713 stars 70 forks source link

avoid warning when missing R cache dir #1129

Closed MichaelChirico closed 9 months ago

MichaelChirico commented 1 year ago

We have a strange setup where tools::R_user_dir() won't be writeable during testing, so normalizePath() throws a warning here. The code works as expected after the warning (cache is just deactivated), so setting mustWork=FALSE here seems reasonable.

OTOH, this might be pretty unique to our testing systems, so if it's too niche an issue we'll just maintain this patch in our fork.

codecov-commenter commented 1 year ago

Codecov Report

Merging #1129 (99d64b2) into main (c38d7f9) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 99d64b2 differs from pull request most recent head 3a13ef6. Consider uploading reports for the commit 3a13ef6 to get more accurate results

@@           Coverage Diff           @@
##             main    #1129   +/-   ##
=======================================
  Coverage   92.35%   92.35%           
=======================================
  Files          46       46           
  Lines        2654     2654           
=======================================
  Hits         2451     2451           
  Misses        203      203

github-actions[bot] commented 1 year ago

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 8f839e601c2c560275344ce9969cbe8932dca661 is merged into main:

:ballot_box_with_check:cache_applying: 200ms -> 201ms [-1.27%, +2.12%]
:ballot_box_with_check:cache_recording: 723ms -> 720ms [-1.17%, +0.4%]
:ballot_box_with_check:without_cache: 1.35s -> 1.36s [-0.08%, +0.63%]

Further explanation regarding interpretation and methodology can be found in the documentation.

lorenzwalthert commented 1 year ago

normalizePath() throws a warning here

In my understanding, a warning is given if the path does not exist. Also, in the help file, searching for writ does not give any results.

So is it non-existant or non-writable?

MichaelChirico commented 1 year ago

So is it non-existant or non-writable?

I think non-extant because non-writable.

lorenzwalthert commented 1 year ago

I think non-extant because non-writable.

Really? Or the other way round?

lorenzwalthert commented 10 months ago

@MichaelChirico happy to help here, can you catch up?

MichaelChirico commented 10 months ago

Sure, what else would you like to see in the PR before merge?

github-actions[bot] commented 10 months ago

This is how benchmark results would change (along with a 95% confidence interval in relative change) if c38d7f98725822061307748ec2eef2c45c5fa25c is merged into main:

:ballot_box_with_check:cache_applying: 177ms -> 179ms [-0.64%, +2.52%]
:ballot_box_with_check:cache_recording: 567ms -> 569ms [-0.23%, +1.14%]
:ballot_box_with_check:without_cache: 1.05s -> 1.05s [-0.6%, +0.42%]

Further explanation regarding interpretation and methodology can be found in the documentation.

lorenzwalthert commented 10 months ago

I think non-extant because non-writable.

I don’t understand that. Can you explain?

MichaelChirico commented 10 months ago

i.e. the process doesn't have permission to write to that directory, so it can't create the expected file(s).

lorenzwalthert commented 10 months ago

Ok thanks. @IndrajeetPatil seems link rot check not working anymore. Can you habe a look?