search

PecanProject / pecan

The Predictive Ecosystem Analyzer (PEcAn) is an integrated ecological bioinformatics toolbox.
www.pecanproject.org
Other
199 stars 231 forks source link

Deprecate `PEcAn.logger` in favor of `log4r`? #3223

Open Aariq opened 9 months ago

Aariq commented 9 months ago

Might be more trouble than its worth at this point, but on the other hand, on a cursory look, it seems like log4r does everything PEcAn.logger does and that would mean one fewer packages to get on CRAN and maintain.

https://github.com/johnmyleswhite/log4r

Sweetdevil144 commented 3 months ago

In fact log4r offers much more used logs other than our PEcAn.logger offers currently.

mdietze commented 3 months ago

I don't have a stong objection, but I also file this under the category of "if it's not broke, don't fix it". There's a number of places in PEcAn where the code is genuinely buggy and unreliable, and I'd personally prioritize those fixes over ones that are mostly aesthetic, especially since this swap could actually introduce new bugs as we learn how to use log4r. Also creates an additional external dependency, which could leave us in a pain point when this package breaks, is deprecated, is upgraded to new syntax, or is dropped by CRAN. All these things have happened to us many time with other packages.

infotroph commented 3 months ago

I agree with Mike. I have some ideas about ways we could improve logging around the edges*, but what we have basically works and any changes to it are low priority.

*In particular: I think it may be possible to have the logger register itself as a condition handler so that it can handle built-in message/warning/stop calls without the package that generates those messages needing to import anything. Slapping together a demo version of that has been low on my list for several years now...