Remove labscript_suite from default config file #45
Conversation
This file contains hidden or 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
chrisjbillington
added a commit
to chrisjbillington/runviewer
that referenced
this pull request
May 23, 2020
…t graceful ctrl-c handling labscript-suite/labscript-utils#45 removes labconfig.config_prefix which we were importing but not using, so we need to remove it. We can remove all version checks as they are now handled by packaging tools. Implement the standard way of ensuring a PyQt application shuts down cleanly upon ctrl-C.
chrisjbillington
added a commit
to chrisjbillington/blacs
that referenced
this pull request
May 23, 2020
And have it named BLACS.h5 instead of containing the hostname. This ensures we don't use variables no longer defined in `labscript_utils.labconfig` as of labscript-suite/labscript-utils#45, whilst also conforming to a more standard layout of where the config file is stored. A nice side effect of storing the BLACS config in the app_saved_configs dir is that if the user changes the experiment name in their labconfig file, they can switch between BLACS configs from different experiments without having to recompile the connection table, change globals files in BLACS, etc. So different experiments on the same machine can have a more self-contained config. I don't see much upside to keeping the hostname in the config filename, so now it's just BLACS.h5. With labconfig there is at least the idea that you might version control the labconfig directory to share changes between computers, but that is not as useful for BLACS' config.
But add it as a default option so that config files may continue to use it as an interpolation value. Saving config files whenever a value is set breaks this, as it actually saves the default value to disk. Without adding a guard against it, it actually writes any provided default values to disk before loading the existing config file, actually wiping out the user's config completely. So this commit removes the overridden methods that save back to disk. No labscript code actually sets config values except for some code in the BLACS plugins `__init__.py`, which adds the list of available plugins. Not writing this back to disk is fine. If we ever make GUI interfaces for manipulating the config file, it is poor form to just overwrite the user's config file anyway, which may re-order sections and remove comments. For that we would want to use something like https://pypi.org/project/ConfigUpdater/. Also simplify the code that gets the config path. There is only one place on disk for config paths now, and if it doesn't exist the user will get a FileNotFoundError with the full path, so I don't think there is a strong need to wrap that error in one written by us that basically says the same thing! This change removes the hostname and config_prefix variables. These variables are imported (unused) by lyse and runviewer, so I will remove them from those projects. They are also imported and used by BLACS to construct its config filepath. I will modify BLACS to instead put its config file in the standard config file locations for apps. Fixes labscript-suite#22
The function that adds userlib and pythonlib to sys.path can't interpolate labscript_suite unless the configparser is given it as a default.
Disable interpolation when creating a config file, otherwise reading entries with interpolation throws an error. We just manipulate the file as raw strings.
chrisjbillington
force-pushed
the
labconfig-remove-profile-key
branch
from
3465041 to
81889b7
Compare
chrisjbillington
added a commit
to chrisjbillington/labscript_utils
that referenced
this pull request
May 29, 2020
These functions save a dict as an `.ini` file and load a dict from an ini file, respectively. Strings are converted with `pprint.pformat` and `ast.literal_eval`, which round-trip for standard Python datatypes. These are the semantics presently used by apps (except BLACS) to save their configs, but they use the `LabConfig` class to do so. Since labscript-suite#45 removed auto-saving of labconfig and auto creation of sections upon setting a value (essentially it made labconfig read-only) apps using labconfig to save data now may fail (labscript-suite/lyse#64). Apps may instead use `save_appconfig()` to save data. They may continue to use labconfig to load data as they are presently, but could be simplified by using `load_appconfig()` instead. This change doesn't modify the format of app config files and so is backward compatible with existing config files.
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.
But add it as a default option so that config files may continue to use
it as an interpolation value.
Saving config files whenever a value is set breaks this, as it actually
saves the default value to disk. Without adding a guard against it, it
actually writes any provided default values to disk before loading the
existing config file, actually wiping out the user's config completely.
So this commit removes the overridden methods that save back to disk.
No labscript code actually sets config values except for some code in
the BLACS plugins
__init__.py, which adds the list of availableplugins. Not writing this back to disk is fine. If we ever make GUI
interfaces for manipulating the config file, it is poor form to just
overwrite the user's config file anyway, which may re-order sections and
remove comments. For that we would want to use something like
https://pypi.org/project/ConfigUpdater/.
Also simplify the code that gets the config path. There is only one
place on disk for config paths now, and if it doesn't exist the user
will get a FileNotFoundError with the full path, so I don't think there
is a strong need to wrap that error in one written by us that basically
says the same thing!
This change removes the hostname and config_prefix variables. These
variables are imported (unused) by lyse and runviewer, so I will remove
them from those projects. They are also imported and used by BLACS to
construct its config filepath. I will modify BLACS to instead put its
config file in the standard config file locations for apps.
Fixes #22
Edit: Above is a typo, actually fixes #35.