-
Notifications
You must be signed in to change notification settings - Fork 29
Implement framework for persistent configuration settings in INI files #15
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
base: master
Are you sure you want to change the base?
Conversation
Implement persistent configuration settings in INI files
Changes made:
* New module at MTpy/config.py (MTpy.config)
The settings are kept in a SafeConfigParser object which you can get by doing:
>>> from MTpy.config import config
For more information on this object see:
http://docs.python.org/library/configparser.html
http://www.doughellmann.com/PyMOTW/ConfigParser/#accessing-configuration-settings
The module attempts to load settings from files in four locations, in this order:
1. Github, but only if necessary
2. $SOURCEFOLDER/MTpy/MTpy.ini
3. $HOME/MTpy.ini
4. ./MTpy.ini
Settings found in later files override those in earlier files, meaning that the user
can set individual settings per directory, or per user, and the defaults are read
from the system's file. An attempt is made to download from Github only if no
INI files can be found locally at all.
|
Sounds like a good plan in general, but I have 2 comments. Again, I've never had these issues, because I try to define things like executables rather locally,or my codes rely on their own config files anyway - and I think this approach is not uncommon. if use_config: I find the name INI confusing/not very intuitive. Cheers, L. |
I am talking about settings which are used in imported modules, such as the
On 15 October 2012 18:27, Geophysics Adelaide notifications@github.comwrote:
|
|
In fact, I disagree with the first point you made. Any settings which users will want to change permanently should never be only contained in the Python script or module, because then when the user updates their system with a new version of mtpy, the changes they intended to be permanent are destroyed. Or if they're using Git, they get a merge conflict. The point of this branch is to allow them to keep those settings on their system somewhere other than the mtpy source code. Don't forget there are still hardcoded default settings -- they are just consolidated into one settings file. We can split configuration settings up into separate files, even a separate one per script or per module but that will make things much more complicated, essentially requiring every script or module to maintain code like that that is in config.py. I don't understand why that would be better. |
|
Mhhh, I think the point is that I do not see the necessity for [Ok ok, veeeery bad example, I admit... please do not use the current So, I am still not sure: is it then optional to rely on the config files On 15/10/12 18:42, nietky wrote:
Dr. Lars Krieger South Australian Centre for Geothermal Energy Research (SACGER) |
|
OK, sorry that we are going in circles here. I'll try to sell it one last time for tonight :-) is it then optional to rely on the config files or will there be an explicit call looking e.g. for executables in there? I don't think it should be compulsory to use the config framework proposed here, but it may simplify code and make it easier for users in many cases. I think we are getting tied up in abstractions like so let's address a concrete example: Every time we run the Occam GUI, the path to the Occam executable is incorrect for me. It is set to That module is obviously generated automatically, so I don't propose changing anything in there. But if this proposal were adopted, by adding the line at https://github.com/geophysics/mtpy/blob/master/mtpy/utils/gui/occam2d/v1/occamgui_v1.py#L48, and adding the following to the proposed and writing a new file to my user directory, the behaviour is changed: now when the GUI is run, the path to Occam is correct for you, and correct for me. For this specific case: is this a problem worth solving? Do you agree that the proposed changes would solve it? Can you propose a better way to go about solving it? in the end, there should not be any pre-defined paths in there - perhaps just a possibility to click "save current settings" Where would the current settings be saved to? How would they be loaded when the GUI is next run? These questions are rhetorical, and I'm not necessarily interested in the actual details of how you would do it -- not a difficult problem. But where would the code go to do this? You would write a couple of functions to save and write the files, and parse them. Call it the save-write-and-parse-settings-code. You would presumably include it in the GUI script. But what happens when we write a new GUI for a different modelling program, or a GUI for BIRRP? These would also have similar problems: paths to executables, perhaps other settings like the choice of coordinate system: lat/longs or UTM. Would we then re-write the save-write-and-parse-settings code into the new GUI? Copy it? It is generally a terrible idea to duplicate significant portions of code. That advice is everywhere. A better solution would be to take that code (to save-write-and-parse-user-settings) and put it in a common place where any future GUIs can access it. That is precisely what this module would provide. Even worse, sometimes our separate GUIs/modules might share the same setting that the user might want to universally set. An example might be a choice between metric and imperial, although I expect we are all SI fans here. Another example: the variables https://github.com/geophysics/mtpy/blob/master/mtpy/processing/birrptools.py#L1085 If this code were changed to: or then no functionality is lost, because anything calling Currently, the function I linked to attempts to find a Futher examples from code of mine that I'll try to work into mtpy some day: https://github.com/nietky/phd/blob/master/phd_modules/oc1d.py#L18 These are all configurable settings that will need a configuration system. Yes, code can automatically assist with establishing them, but such code is initialisation code, and belongs in Further examples: configuring default plot styles. Default coordinate systems. Anything you can think that makes sense with 'default' ahead of it. |
|
save current settings OK I realize this branch lacks a few features, so I will add them:
That should provide all the functionality you'd need for a 'save settings' option in the Occam GUI. |
|
I agree with everything Kent has described in detail. On Tue, Oct 16, 2012 at 9:22 AM, nietky notifications@github.com wrote:
|
The changes on this branch are meant as a way to tackle all the hardcoded paths that are around. The idea is to basically have all settings that users might want to permanently change per project in an INI file. We really don't want users (including ourselves) changing the source directly for convenience, because then it's reversed every time you update the code.
Can you take a look at it, and let me know what you think of this approach? Should we do it this way? Do you understand my explanation?
So this is how my changes work:
The code that sets everything up is in mtpy/config.py
Settings are in one or more INI files. There is a 'master' INI file in the repository, at mtpy/config/mtpy.ini. Users can place their own INI files in their $HOME directory, or in the current working directory. The two latter INI files need not have all the settings in them, because
config.pywill look at each one in turn, overwriting the settings with those it finds in more 'local' INI files. So the 'master' INI has the lowest priority, and the current working directory INI has the highest priority. It doesn't matter if there are no INI files in the $HOME directory or the working directory.When config.py is imported it loads the INI file into a
SafeConfigParserobject, which is a bit like a dictionary, and makes it available as theconfigobject in its module namespace. So to access this object from anywhere inmtpycode, use:Changes can't be made during an interpreter session. This might be modified later, but I thought we should just keep it simple for now. The
configobject is a bit like a dictionary, but not as nice. It has separate sections, which contain name/value pairs, but to get at the values you need to call theget(section, name)method. So for an INI file that looks like:to get at the settings:
This is awkward, but I think it is the best cross-platform way to do things. For example, in some module somewhere in
mtpyyouwould do the following:
The module that this is based on (ConfigParser) is in the standard library. There are other and better choices for doing configuration, but none of them are standard library modules, and they're only slight improvements. I think we are better off sticking with an option that everyone has installed by default. More information about how
SafeConfigParserobjects work is here:http://docs.python.org/library/configparser.html
http://www.doughellmann.com/PyMOTW/ConfigParser/#accessing-configuration-settings
For now, I haven't actually added any settings. I just wanted to propose this as a framework before going ahead and adding settings everywhere.
Also, a minor point, but if for some reason the
mtpy.inifile in the source folder is missing or broken for some reason,config.pywill try to download the latest version from Github.So... does that make sense? Do you like it?