larmarange commented 1 year ago

.get_package_dependencies() should be base on packageDescription() as installed.packages() could be time consuming if there are too much installed packages

larmarange commented 1 year ago

Adding .get_all_packages_dependencies() to keep a version based on installed.packages()

larmarange commented 1 year ago

@ddsjoberg maybe you could have a loot at it

larmarange commented 1 year ago

It looks quicker

library(broom.helpers)
test <- bench::mark(
  a = .get_package_dependencies("broom.helpers"),
  b = .get_all_packages_dependencies("broom.helpers"),
  check = FALSE
)
test
#> # A tibble: 2 × 6
#>   expression      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 a            17.8ms   20.4ms      46.2    5.75MB     23.1
#> 2 b            44.5ms   50.4ms      19.3     4.3MB     28.9
ggplot2::autoplot(test)

^{Created on 2022-09-25 with reprex v2.0.2}

codecov[bot] commented 1 year ago

Codecov Report

Merging #178 (34b7673) into main (27a6153) will decrease coverage by 1.60%. The diff coverage is 92.00%.

Additional details and impacted files

[![Impacted file tree graph](https://codecov.io/gh/larmarange/broom.helpers/pull/178/graphs/tree.svg?width=650&height=150&src=pr&token=onefg3YI04&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange)](https://codecov.io/gh/larmarange/broom.helpers/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange) ```diff @@ Coverage Diff @@ ## main #178 +/- ## ========================================== - Coverage 98.90% 97.30% -1.61% ========================================== Files 38 38 Lines 1643 1667 +24 ========================================== - Hits 1625 1622 -3 - Misses 18 45 +27 ``` | [Impacted Files](https://codecov.io/gh/larmarange/broom.helpers/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange) | Coverage Δ | | |---|---|---| | [R/assert\_package.R](https://codecov.io/gh/larmarange/broom.helpers/pull/178/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange#diff-Ui9hc3NlcnRfcGFja2FnZS5S) | `59.09% <92.00%> (-40.91%)` | :arrow_down: |

ddsjoberg commented 1 year ago

@larmarange awesome!

do you mind if a take a look in a month or so? I've got so many packages to update with tidyselect changes, and it's my super busy season this month at work.

larmarange commented 1 year ago

No pb. There is no emergency

larmarange commented 1 year ago

oups ! I have merged the PR while I was thinking updating the PR from the main branch.

larmarange commented 1 year ago

However, it does not induce major changes

ddsjoberg commented 1 year ago

hey hey @larmarange !

I didn't get a chance to review this (I am sure it's all good!).

But I wanted to let you know that an update to purrr is coming that has a breaking change in broom.helpers. Check out the first line in this issue. https://github.com/tidyverse/purrr/issues/969

The update you just made may have solved this. But it's something we'll need to investigate soon-ish.

larmarange commented 1 year ago

Thanks. I will try to explore it

larmarange commented 1 year ago

Hi @ddsjoberg Could you have a look at https://github.com/larmarange/broom.helpers/pull/181

It should fix the problem with purrr

larmarange / broom.helpers

trying to improve `.assert_package()` #178

Codecov Report