Enable configfile specification for mock_snakemake #1135
Conversation
There was a problem hiding this comment.
Great again @yerbol-akhmetov !!!
I've added a minor comment; I'd merge the PRs with squash and merge if it is ok for you as it would leave flexibility for the merge issue that is open.
scripts/_helpers.py
Outdated
| if isinstance(configfile, str): | ||
| with open(configfile, "r") as file: | ||
| configfile = yaml.safe_load(file) | ||
| else: |
There was a problem hiding this comment.
Rather than else, maybe "elif not isinstance(configfile, dict)" or drop the if case?
By default it is none, so the else case may not be necessary but a dictionary may be provided as well as input.
What do you think?
Almost ready to merge :D
There was a problem hiding this comment.
@davide-f, thanks for the feedback. I have initially meant to omit else case, because default if None. But then I thought what if the user gives something other than str, it can cause error. Therefore I have added additional else with None. Similar thing I have noticed in PyPSA-Eur. Maybe it is better then drop else case?
There was a problem hiding this comment.
@davide-f, thanks for the feedback. I have initially meant to omit else case, because default if None. But then I thought what if the user gives something other than str, it can cause error. Therefore I have added additional else with None. Similar thing I have noticed in PyPSA-Eur.
Let's drop it :) that function is not used by the normal user.
If provided a wrong input, probably it is safe for the code to through an error rather than silently fix it.
Great contribution!
There was a problem hiding this comment.
@davide-f, ok, thanks, understood.
There was a problem hiding this comment.
I have dropped else condition, while keeping if, so to make sure, config is read only if the path is given as string.
|
@yerbol-akhmetov apologies, by merging the other PR this has let to conflicts. Can you fix it? |
Thanks, @davide-f. I have resolved the merge conflicts and updated the branch with main. |
…#1135) * enable configfile for mock_snakemake * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add release notes * drop else condition --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…#1135) * enable configfile for mock_snakemake * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add release notes * drop else condition --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…#1135) * enable configfile for mock_snakemake * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add release notes * drop else condition --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Changes proposed in this Pull Request
Good day. Here I propose to enable specification of configfile while using
mock_snakemake. It is particularly useful when debugging the scenario with its own run name. Currently, the files are searched inresources/ornetworks/folders as example. If user had already prepared files in folders under run name, it is not visible. Overwriting config file is enabled in snakemake<8 with overwrite_config parameter, which takes dictionary.Checklist
doc/release_notes.rstis amended in the format of previous release notes, including reference to the requested PR.