search

jacob-long / jtools

Tools for summarizing/visualizing regressions and other helpful stuff
https://jtools.jacob-long.com
GNU General Public License v3.0
165 stars 22 forks source link

Dependencies question for JOSS review #149

Closed misken closed 4 months ago

misken commented 7 months ago

Hi @jacob-long, I'm one of the reviewers for your recently submitted JOSS paper. I'll be opening issues here regarding the review - which is at https://github.com/openjournals/joss-reviews/issues/6610.

I installed jtools in an R virtual env and was just reproducing the examples from the GitHub readme page. That page does mention that various jtools functionality depends on sandwich and huxtable. I attempted running the code before installing these and got appropriate error messages. After installing huxtable and rerunning the example using export_summs, then I got an error message saying I needed to install brooms. After installing brooms, that example worked. Further down, when trying plot_summs, I got the correct output but got errors in the console complaining about needing brooms.mixed. I see that these are all suggested packages, so I'm not saying these are bugs or problems. Just pointing out that some R newbies might get hung up on stuff like this. I'm not an R package dev so not really sure what the best practice is for handling this - maybe you're already doing what is suggested by CRAN.

gaborio commented 7 months ago

I was actually going to open a discussion about this, I use the package in my class and I forgot that in my machine I have those dependencies installed, but my students don't so they encountered errors. What is the logic for making them suggestions instead of dependencies?

jacob-long commented 7 months ago

Thanks for raising this. These are, to some extent, decisions made long ago and not often re-examined so it's helpful to hear when/if they cause trouble for somebody. The initial reasoning was fairly straightforward in that I hoped to avoid "too many" hard dependencies and the functionality these were providing was probably not as core to the package as they may seem now.

That being said, my reaction to the issue is that probably all of these packages should be imports rather than suggests, with the possible exception of huxtable. I certainly aim to make this package particularly accessible to people who are relatively unfamiliar with R/programming and package management isn't going to be second nature to those folks — and I'm already making some other design choices that make things a little inefficient in the service of that audience so this would be consistent with that approach.

I will take a look at this soon and (most likely) make some changes.