Features/cli app #1
Conversation
…o features/cli-app
…o features/cli-app
…o features/cli-app
There was a problem hiding this comment.
This is what I found so far. Only minor things, nothing big.
A few more general notes:
- Maybe we can drop
download_rawdata.pyand move the single function call to the cli? scriptsis a rather general name that doesn't convey any meaning. How about moving the modules below it to the top or find places for them with more speaking names.- Is there a way to merge the two configuration files? Maybe by having a
scenariosection?
But bus.py, capacity_factors.py, compute.py, electricity.py and grid.py are rather long and not exactly easy to figure out. So I'll have to spend more time reading them to figure out whether there's a necessity to refactor them.
src/fuchur/cli.py
Outdated
| directories=["data", "resources"] | ||
| ) | ||
|
|
||
| datapackage.building.initialize_datapackage( |
There was a problem hiding this comment.
All those calls having nearly the same signature could potentially be written shorter. But they also hint at a refactoring potential. Don't know whether any of this would really be an improvement though. The first option would be shorter, but harder to read. For refactoring I would have to check out the functions in question.
There was a problem hiding this comment.
I dont know what you mean...
| @click.pass_context | ||
| def construct(ctx, config): | ||
| config = datapackage.building.read_build_config(config) | ||
| _construct(config, ctx) |
There was a problem hiding this comment.
I'd avoid those underscore methods. If you don't want to clutter the fuchur.cli with these long functions, put them in another module.
Also, you don't have to import as _compute to avoid the name clash. If you just do import fuchur.scripts.compute, the module will only be available under its fully qualified name. ;)
There was a problem hiding this comment.
You can change this if you like
Co-Authored-By: simnh <[email protected]>
Co-Authored-By: simnh <[email protected]>
I saw the `allow_failures` key was on the same level as `matrix`, instead of being subordinate to it. The way it is now should replicate the example. Hope it works.
Two is the number of empty lines you should put after your imports and the number of empty lines you should put after your imports is two. Otherwise you will never defeat the Killer Rabbit of Caerbannog. ;)
Now you can specify built in scenarios by their `name` when running `fuchur construct`.
This is done by implementing the `Scenario` class, which has the
following features:
* It encapsulates loading scenario from a path via the `from_path`
class method and
* it looks for a `parents` key in the loaded scenario, which enables
copying other scenarios into this one.
The `from_path` method does everything that is necessary to load a
scenario from a file. This currently entails:
* using a special function from `oemof.tabular` instead of just using
`toml.load` and
* registering the loaded scenario as known under `fuchur.scenarios`
under its name and its path.
A scenario's `parents` can be specified as a list of strings, which will
be looked up in `fuchur`'s known scenarios. If they string is not found,
it is tried to be loaded via `Scenario.from_path`.
You can now specify builtin scenarios simply by their `name` when calling `fuchur construct` and you can copy other scenarios into a scenario by referring to the former via a `parents` entry in the latter.
…d update by children
No description provided.