-
Notifications
You must be signed in to change notification settings - Fork 1
Use Hierarchical Machine Config #711
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
Merged
Merged
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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #711 +/- ##
==========================================
+ Coverage 40.12% 40.30% +0.18%
==========================================
Files 99 99
Lines 11577 11602 +25
Branches 1547 1553 +6
==========================================
+ Hits 4645 4676 +31
+ Misses 6715 6708 -7
- Partials 217 218 +1 🚀 New features to boost your workflow:
|
… replaced it with 'get_machine_config' from 'murfey.util.config' instead
…nstrument machine configs hierarchically; it can load all the machine configs or just the specified one
… as the 'from_file' formerly in 'murfey.util.config'; revert the change
4ad0236 to
85d91c9
Compare
85d91c9 to
d5120a7
Compare
…rom_file' by proxy as well
stephen-riggs
approved these changes
Dec 9, 2025
Contributor
stephen-riggs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks for doing this
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.
The master machine config YAMl file used by our organisation has grown to the point where it becomes easy to miss updating lines for certain instruments when changes are made to Murfey. In order to mitigate this, a hierarchical machine config structure was proposed, in which shared keys can be grouped together under common and instrument-specific dictionaries, and only keys unique to each instrument (i.e. IP addresses, file paths, etc.) need to be added to the individual instrument dictionaries.
This PR implements the necessary changes to
machine_config_from_file(), allowing it to read from the general dictionary, followed by the instrument shared dictionary, before loading up keys from the individual instrument dictionaries. It also updates standardises how the machine config is called throughout the repo, and removes duplicate functions that end up serving a similar purpose.Another key change introduced in this PR is that all keys in the machine config are now fully optional, with default values being set for all of them. This makes it easier for us to instantiate the class. The code base was updated so that it will correctly replace these newly optional keys with default values if none were passed through.