[Concept] Refactor the way tasks access the config - #888
Draft
mranst wants to merge 15 commits into
Draft
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
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.
Description
For a few reasons, I've never really loved the way that Swell handles accessing config values from tasks. The way that the config object isn't rendered unless the task itself it run means there's a disconnect in the source code of the task, where you don't necessarily know if you're accessing the right name when you're writing it. The fact that keys in experiment.yaml are being assigned as methods to an object on the fly is a little confusing, and this behavior is buried under a few layers of obfuscation.
So, I've been trying out this concept that is slightly different, where config accesses instead make direct references to questions in
question_defaults.py.self.config.bundles(default=bundles) -> self.config.resolve(qd.bundles, default=bundles)I like this because I believe it improves the situation I described above. You can now directly see where keys are being defined. It also allows you to see docstrings in the task from an IDE, which can improve readability:

This PR (which is mostly a proof of concept at this point, though it partially works) also does a few other things, including type checking for keys in
experiment.yamlfrom the config, and getting rid of the cli.There are a few cons to this, including:
So I wanted to put this out there to see if people think this is a worthwhile direction to follow.