Issue/217 data source configs

[peterdudfield]

Pull Request

Description

• Change configuration to be more module on each input data source
• add default forecast minutes and historic minutes

Warning this is a break change for the confiiguration model

Fixes #217

How Has This Been Tested?

• Unittest

• ran prepare_ml_data.py script dummy run, ot check it works

• No

• Yes

Checklist:

• My code follows OCF's coding style guidelines
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• I have checked my code and corrected any misspellings

[JackKelly]

This is looking really good! Thank you!

And a really nice and elegant implementation of the default_history_minutes thing, too!

One way in which this change could make our code considerably simpler is if the names of the config fields precisely align with the argument names in the DataSource constructor.

So, for example, in the config model, we could change nwp_channels and satellite_channels to just channels. Or, alternatively, we could change the DataSource subclasses... specifically, we could change SatDataSource and NWPDataSource to take the arguments satellite_channels and nwp_channels. (I'd probably slightly lean towards the second option, because I like the verbose and explicit field names that you've chosen for the config fields! Although I'm very happy to be persuaded otherwise! Changing the DataSource sub-class constructor arguments could be a subsequent PR, though 🙂 )

Thanks again!

[JackKelly]

The value of these consts also needs changing near the top of this file, I think?

[peterdudfield]

Yes, they do - thank you

[peterdudfield]

This is looking really good! Thank you!

And a really nice and elegant implementation of the default_history_minutes thing, too!

One way in which this change could make our code considerably simpler is if the names of the config fields precisely align with the argument names in the DataSource constructor.

So, for example, in the config model, we could change nwp_channels and satellite_channels to just channels. Or, alternatively, we could change the DataSource subclasses... specifically, we could change SatDataSource and NWPDataSource to take the arguments satellite_channels and nwp_channels. (I'd probably slightly lean towards the second option, because I like the verbose and explicit field names that you've chosen for the config fields! Although I'm very happy to be persuaded otherwise! Changing the DataSource sub-class constructor arguments could be a subsequent PR, though 🙂 )

Thanks again!

Yea I would vote to keep the variables verbose, and change the DataSource in a subsequence PR. What do you think?

Thanks for your comments

[JackKelly]

Yea I would vote to keep the variables verbose, and change the DataSource in a subsequence PR. What do you think?

Sounds good to me! Thanks!