hubverse-org / hubEnsembles

Ensemble methods for combining hub model outputs.
https://hubverse-org.github.io/hubEnsembles/
Other
6 stars 2 forks source link

Code review from Hugo #121

Closed Bisaloo closed 1 month ago

Bisaloo commented 1 month ago

As promised a couple of months ago when reviewing the manuscript.

Overall, the codebase seems in great condition to me so I didn't have much changes to propose.

Here is a summary of the minor changes or fixes:

lshandross commented 1 month ago

@elray1 what do you think about using the magrittr vs base R pipe? Is our stance still the same as on that issue, i.e. the former is preferable to avoid the dependency on R 4.1?

elray1 commented 1 month ago

I agree with Hugo's comment -- using the magrittr pipe already felt like a borderline decision a couple of years ago, and i'm good with moving on. (Actually, reading more carefully, maybe I got Hugo's comment backwards. But regardless, I think that at this point I'd vote to standardize on the base R pipe)

lshandross commented 1 month ago

Ok, I can make the changes to update the pipe to the standard one, then merge this pull request