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

Adding design notes on core data setup and configuration #65

Merged
merged 4 commits into from
Sep 2, 2022

Conversation

davidorme
Copy link
Collaborator

This pull request provides a proposal for the data initialisation process.

Fixes #64

Type of change

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist

  • Make sure you've run the pre-commit checks: $ pre-commit run -a
  • All tests pass: $ poetry run pytest

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2022

Codecov Report

Merging #65 (f737745) into develop (08fd3e9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop      #65   +/-   ##
========================================
  Coverage    75.86%   75.86%           
========================================
  Files            3        3           
  Lines           58       58           
========================================
  Hits            44       44           
  Misses          14       14           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@jacobcook1995 jacobcook1995 left a comment

Choose a reason for hiding this comment

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

Made a couple of comments, also am wondering whether this actually should close issue #64, which it currently will automatically when merged

docs/source/development/design/core.md Show resolved Hide resolved
docs/source/development/design/core.md Show resolved Hide resolved
@davidorme
Copy link
Collaborator Author

That's a good point about closing - I intended the issue just to provide the proposal for discussion, but it might as well serve to cover the implementation.

This would have been better as a GH discussion!

configuring the data generator with a set random number generator seed.

These could get arbitrarily complex - so at some point we should just say, if you want
sufficiently complex generated data, just roll your own NetCDF files!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please explain this a bit more? When would this apply?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hard to define! But if someone wanted to simulate a scenario of 5 drought years followed by 3 years of elevated rainfall then an El Niño and then a plague of frogs - that's probably outside the scope 😄

Copy link
Collaborator

@vgro vgro left a comment

Choose a reason for hiding this comment

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

Looks very good, clear starting point to design the core set up and config.
Maybe we can start a discussion for more details on data inputs types/format and grid shapes?

Copy link
Collaborator

@vgro vgro left a comment

Choose a reason for hiding this comment

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

Thanks for adding examples and clarification ( I'm just making sure it's idiot proof ;-) ). I think the document is very clear now, ready to merge from my point of view 👍

@davidorme davidorme merged commit d879d33 into develop Sep 2, 2022
@davidorme davidorme deleted the feature/core_data_notes branch September 2, 2022 14:34
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.

Proposal for data configuration system and data generation
4 participants