Initial version of updated README #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sumodm
commented
Jun 10, 2020
sumodm
commented
- Added notes on repo organization
- Added notes on adding new notebooks and code to src (prod)
- Added basic notes on workflow
1. Added notes on repo organization 2. Added notes on adding new notebooks and code to src (prod) 3. Added basic notes on workflow
smerugu
reviewed
Jun 10, 2020
smerugu
reviewed
Jun 10, 2020
smerugu
reviewed
Jun 10, 2020
smerugu
reviewed
Jun 10, 2020
smerugu
reviewed
Jun 10, 2020
smerugu
reviewed
Jun 10, 2020
| 1. We follow [git-flow](https://nvie.com/posts/a-successful-git-branching-model/) with staging in our parlance being develop in theirs. | ||
| 2. Ensure that your code satisfies [pep8](https://www.python.org/dev/peps/pep-0008/) | ||
| 3. Ensure that you have documentation as per [pep257](https://www.python.org/dev/peps/pep-0257/) | ||
| 4. TODO: Decide PEP257, PEP287, something else. |
There was a problem hiding this comment.
It's task reminder for me to decide which docstring format we want to use.
smerugu
reviewed
Jun 10, 2020
smerugu
reviewed
Jun 10, 2020
smerugu
reviewed
Jun 10, 2020
| |-- training_module.py : Training module, this currently used hyperopt to optimize | ||
| |-- data_fetcher_module.py : Data fetcher module for fetching various data into requisite format | ||
| `-- scenario_forecasting_module.py : Scenario based forecasting module. | ||
| `-- utils : Various tools like loss_funcs, hyperopt wrapper etc |
There was a problem hiding this comment.
Readme description is fine but directory requires cleanup
smerugu
reviewed
Jun 10, 2020
| 4. TODO: Need to expand this and verify this. | ||
|
|
||
| ## Adding new model wrapper to Production (src) | ||
| 1. If you need new parameters, then update that in entities in forecast_variables. |
There was a problem hiding this comment.
Did you mean new time series variables as inputs (parameters sounds like model parameters)?
smerugu
reviewed
Jun 10, 2020
| 2. If you are adding new model | ||
| a. Then add that in model_class.py | ||
| b. Create your new_model.py in model_wrappers and update the model_factory | ||
| 3. if you want this model to be intervention enabled, then write the wrapper that maps variables call it intervention_enabled_your_model.py and place in model_wrappers. |
There was a problem hiding this comment.
Yes, I understand Anupama's point. This intervention_enabled_X is clunky. Next round of refactoring, let us think through and fix it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.