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

Configuration management: Allow for creating / saving / loading configuration on the fly. #23

Merged
merged 34 commits into from Jul 30, 2019

Conversation

euronion
Copy link
Collaborator

@euronion euronion commented Jul 22, 2019

After doing some overengineering on the config management, now a adequately pythonic solution.

The config.py module provides the module for all other modules via

from . import config 

Changes to objects of the module can either be directly, e.g.

config.cutout_dir = "my/cutout/dir"

Or via a dictionary update:

config.update({"cutout_dir" : "my/cutout/dir"})

Config can be read and written with config.read(p) and config.save(p).
The config file is in yaml style and a default config is provided.

Relative and absolute paths are supported, for this a new auxiliary function is introduced in utils.py:
construct_filepath(path) reads a path and - if it is a relative path - returns the corresponding absolute path. Relative paths are considered relative to the directory of the currently in use configuration file via config.config_path.
If this variable is also not set, then the relative file is returned unchanged (i.e. relative to current directory).
Any absolute path is immediatley returned unchanged.

NB: Need to squash when merging.

Fixes #4.

The config module defines a new config class and implements a config object to be shared globally across modules.

The .yaml file prototype contains the supported structure (yaml) and variables including defaults.
Constructor now returns empty objects if neither dictionary nor path are provided.
The default locations are only searched for and evaluated when the module is first imported.
Include instructions.
Change the config to work as a singelton (module) instead of a class and objects.
The config is now really shared among modules, instead only of a copy of the same object.
This reverts commit d67cb2e.
Copy link
Member

@coroa coroa 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 the PR, i have a few comments from a first read-through.

Hmm .. the module variable scheme you are using, means one cannot use two different cutouts with separate config settings (which is probably not necessary).

I have to think about your proposal (and its implications) to use the config path as base directory for all data access.

README.rst Show resolved Hide resolved
atlite/config.py Outdated Show resolved Hide resolved
atlite/cutout.py Outdated Show resolved Hide resolved
atlite/resource.py Show resolved Hide resolved
examples/create_cutout.py Outdated Show resolved Hide resolved
examples/create_cutout.py Outdated Show resolved Hide resolved
@euronion
Copy link
Collaborator Author

Thanks for the PR, i have a few comments from a first read-through.

Thanks for the remarks!
Do I need to do something special to accept your code suggestions?
Somehow the field which all the tutorials say I should use is greyed out... .
(atm I am including all resolved remarks into my local branch and will push the changes when everything is resolved).

Hmm .. the module variable scheme you are using, means one cannot use two different cutouts with separate config settings (which is probably not necessary).

Correct. I was toying for literally a week with different approaches and looking at different best practices.
I first also went for a class / object configuration management system, but it is
a.) Not considered pythonic by many people.
b.) An unreasonably too complex piece of code.
c.) Quite sensible to errors from users and coders.
d.) Does not provide any significant added value from my PoV compared to using an config file or using an config dictionary which is then applied via the config.update(dict) method.

I have to think about your proposal (and its implications) to use the config path as base directory for all data access.

The choice for the base directory for relative paths was hard, but this makes IMHO most sense, as I would expect users (read: myself) to place a custom configuration if I need it inside the project-directory next to the cutouts and resources like turbine configs.

But that was also the idea of implementing the utils.construct_filepath(...) to handle the relative paths independent of any specific choice.

@coroa
Copy link
Member

coroa commented Jul 25, 2019

After resolving the remaining todo's I will review it a second time and I think it's good to go then. Great!

Copy link
Member

@coroa coroa left a comment

Choose a reason for hiding this comment

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

I like that config.py became even simpler in the last iteration.

Just a few cosmetic changes and this is good to go!

atlite/config.example.yaml Outdated Show resolved Hide resolved
examples/create_cutout.py Outdated Show resolved Hide resolved
atlite/cutout.py Outdated Show resolved Hide resolved
@euronion
Copy link
Collaborator Author

Changes included.
Happy to hear you liked the simpler config.py :) .

@coroa coroa merged commit 6039a70 into PyPSA:v0.2 Jul 30, 2019
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.

None yet

2 participants