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

Fix/post angola #143

Merged
merged 21 commits into from
Sep 20, 2022
Merged

Fix/post angola #143

merged 21 commits into from
Sep 20, 2022

Conversation

rosannaamato
Copy link
Contributor

Issue #137 suggests 17 fixes / improvements identified following delivery in Angola, this branch addresses 9 of them (listed below). Comments added to #137 describe which commits address specific improvements.

  • Inconsistent location for location of data (historical in WS3 but not WS2)
  • 4.2c solution is for pr not tm and also only historical not future as well
  • Wet days - using threshold of 0 not 1 -replace custom aggregator with count > 1
  • Future dir should be explicitly rcp85
  • Mean over season should be over time (this should remove redundant time dimension)
  • Inconsistent on use of REMO2015 in filenames
  • Needs to be made VERY clear when we expect something will fail (worksheet 1 - merge problem)
  • Remove f strings and remaining .format (just use concatenation)
  • set domain name as variable at start.

Changes are applied to EAS-22 worksheets 1-6 + solutions, testing was undertaken using data in /project/ciid/projects/PRECIS/worksheets/data_v2/EAS-22.

Please review changes.

Copy link
Collaborator

@nhsavage nhsavage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apart from the addition of the files in .idea (which I have taken out of the branch) these changes seem great to me. Thanks for your efforts.

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

Successfully merging this pull request may close these issues.

2 participants