Skip to content

Do not fail if a namespace cannot be loaded.#4

Merged
gaborcsardi merged 1 commit intor-lib:masterfrom
jimhester:handle_namespace_failures
Aug 2, 2017
Merged

Do not fail if a namespace cannot be loaded.#4
gaborcsardi merged 1 commit intor-lib:masterfrom
jimhester:handle_namespace_failures

Conversation

@jimhester
Copy link
Copy Markdown
Member

This fixes devtools failures when using sessioninfo

https://travis-ci.org/hadley/devtools/jobs/260288634#L823-L827

@gaborcsardi
Copy link
Copy Markdown
Member

Thanks! Wouldn't it be better no to try to load it at all?

The point of using getNamespaceVersion was to detect the rare case when the loaded version is different from the on-disk version. If the package is not loaded, we can just keep the loaded version empty.

Anyway, I can merge this now, and change it later.

@jimhester
Copy link
Copy Markdown
Member Author

Maybe, I can do that instead, one sec, will update the PR.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 2, 2017

Codecov Report

Merging #4 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #4   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      8           
  Lines         173    174    +1     
=====================================
+ Hits          173    174    +1
Impacted Files Coverage Δ
R/utils.R 100% <100%> (ø) ⬆️
R/loaded-packages.R 100% <100%> (ø) ⬆️
R/dependent-packages.R 100% <100%> (ø) ⬆️

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 8de1163...3806e7c. Read the comment docs.

@jimhester jimhester force-pushed the handle_namespace_failures branch from 96332d3 to c15817f Compare August 2, 2017 19:23
@jimhester
Copy link
Copy Markdown
Member Author

Ok updated to only get namespace versions for loaded namespaces.

@gaborcsardi
Copy link
Copy Markdown
Member

Are the NAs printed? How does the printed output look?

@gaborcsardi
Copy link
Copy Markdown
Member

I'll update the mocked test, don't worry about that.

@jimhester
Copy link
Copy Markdown
Member Author

Hmm NA actually causes errors from base::package_version() because it is not a valid package version, as done "". "0.0.0" works, but the output is kind of gross.

─ Packages ───────────────────────────────────────────────
 package           * version    date       source

 acepack             0.0.0 (!)  2016-10-29 CRAN (R 3.4.0)

 AnnotationDbi       0.0.0 (!)  2017-04-25 Bioconductor

 ape                 0.0.0 (!)  2017-02-14 CRAN (R 3.4.0)

 archive             0.0.0 (!)  2017-07-28 Github (jimhester/archive@a203312)

 assertive.base      0.0.0 (!)  2016-12-30 CRAN (R 3.4.0)

 assertthat          0.2.0      2017-04-11 CRAN (R 3.4.0)

@gaborcsardi
Copy link
Copy Markdown
Member

Yeah, I am afraid we need to change the print method.

@gaborcsardi
Copy link
Copy Markdown
Member

Somewhere around here:

badloaded <- package_version(x$loadedversion) !=

If it is not loaded, just use the on-disk version.

@jimhester jimhester force-pushed the handle_namespace_failures branch from c15817f to 3806e7c Compare August 2, 2017 19:52
@jimhester
Copy link
Copy Markdown
Member Author

Ok turns out if you use package_version(NA, strict = FALSE) we get an NA package version, which we can then handle in the print method, which looks nicer and should still catch the case where a loaded version is different than the installed version as before (3806e7c).

@gaborcsardi
Copy link
Copy Markdown
Member

Great! Thanks!

@gaborcsardi gaborcsardi merged commit be05f95 into r-lib:master Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants