-
Notifications
You must be signed in to change notification settings - Fork 20
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
Removing pasture suitability calibration from calcPastureSuit.R from the preprocessing #41
Conversation
…ster_mppalves merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Marcos,
that looks all really good :) I added some comments.
I guess the only think where I would ask you to check is LN:96, the rest is kind of optional.
cool that you found time to fix this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the reordering and cell renaming looks suspicious to me and should be removed if possible. Rest looks good to me
- fixed various linter warnings
adjusted package to mrcommons split
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. Just fix the merge conflicts and build again and you should be from my perspective ready to go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had a quick look if my comments have been addressed and it looks very good :)
Thanks Marcos for addressing everything. Good to go from my side :)
comply with new convention to have column names "region", "country", "superregion" in regionmappings
Changes summary: