Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code comments #11

Open
jrising opened this issue Jan 27, 2025 · 0 comments
Open

Code comments #11

jrising opened this issue Jan 27, 2025 · 0 comments

Comments

@jrising
Copy link

jrising commented Jan 27, 2025

openjournals/joss-reviews#7538

Comments on code in R:

  • d$plant <- ifelse(d$Plant.start < d$Plant.end, ((d$Plant.start + d$Plant.end) / 2 / 30), ((d$Plant.start + d$Plant.end + 365) / 2 / 30))
    : Day-of-year divided by 30 isn't a great approximation for month, unless you're working with 360-day calendar data.
  • # Deal with exceptions (countries by states or regions)
    : This would be less error-prone if encoded in a data file.
  • input_file = "crop_calendar.csv"
    : If you intend for users to modify this, provide them a way to use their own filename.
  • pattern = utils::glob2rx("*precip*rfc*"),
    : Why is only the RFC weighted data used?
  • dplyr::left_join(mirca_harvest_area, by = c("lon", "lat")) %>%
    : This assumes a 1-to-1 mapping between region and grid cell. At the country level, this is usually fine, but would not work at higher resolutions.
  • reg$se_robust <- sandwich::vcovHC(reg, type = "HC", weights = iso)
    : The SE should also be clustered, since there will be correlation between error terms in the each country.
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

No branches or pull requests

1 participant