-
Notifications
You must be signed in to change notification settings - Fork 211
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
Update Snakemake and adapt for dag #1312
base: main
Are you sure you want to change the base?
Conversation
Major changes: - update Snakemake in environment.yaml to version 8 Minor changes: - move the input file logging from build_demand_profiles.py to retrieve_databundle_light.py to prevent issues with the creation of the rule graph
for more information, see https://pre-commit.ci
@davide-f moving the logging to retrieve_databundle_light required a few more changes, since the previous version of |
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.
Many thanks for the PR!
I've added minor comments below.
Many thanks!
scripts/build_demand_profiles.py
Outdated
return load_paths, load_dir, file_names | ||
|
||
|
||
def get_load_paths_gegis(ssp_parentfolder, config): | ||
""" | ||
Return load paths for GEGIS outputs. | ||
""" | ||
|
||
load_paths, _, _ = prepare_load_paths_gegis(ssp_parentfolder, config) | ||
|
||
return load_paths | ||
|
||
|
||
def log_load_paths_gegis(ssp_parentfolder, config): | ||
""" | ||
Log the load paths for GEGIS outputs. | ||
""" | ||
|
||
load_paths, load_dir, file_names = prepare_load_paths_gegis( | ||
ssp_parentfolder, config | ||
) | ||
|
||
logger.info( | ||
f"Demand data folder: {load_dir}, load path is {load_paths}.\n" | ||
+ f"Expected files: " | ||
+ "; ".join(file_names) | ||
) |
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.
Thanks for trying to fully preserve the previous behavior.
We can keep it simple and drop the logger here
scripts/retrieve_databundle_light.py
Outdated
@@ -95,6 +95,7 @@ | |||
create_logger, | |||
progress_retrieve, | |||
) | |||
from build_demand_profiles import log_load_paths_gegis |
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.
Mmm on second thought, maybe it is not well placed here but rather in build_demand profiles.
By looking at the function build_demand_profiles in build_demand rule, there is:
logger.info(f"Merging demand data from paths {load_paths} into the load data frame")
that should already explain the loaded load paths, so probably this should be enough.
As the CI will be working, we can check it here online.
So here probably we could restore the previous behavior.
Minor changes: - Restrict the snakemake-minimal version to avoid Snakemake issue #3202 Co-authored-by: Davide Fioriti <[email protected]>
This reverts the changes to build_demand_profiles and retrieve_databundle_light in commit 5baa844.
Minor changes: - list matplotlib restrictions as they are not required anymore after the pypsa Update
Minor changes: - remove the outdated HTTPRemoteProvider in the Snakefile and change it to the corresponding Snakemake storage plugin - include snakemake-storage-plugin-http in envs/environment.yaml - rename the update parameter in the Snakefile and in scripts/prepare_gas_network.py since update is restricted - change the wildcard strings in the Snakefile to raw strings since escape characters in strings are deprecated - change the regular expression strings in scripts/prepare_network.py to raw strings since escape characters in strings are deprecated
Minor changes: - remove the loggin when initializing Snakemake from scripts/build_demand_profiles.py
@davide-f Thanks for the review! |
Here is a list of changes I found neccessary to fully adapt the code to Snakemake 8:
|
Minor changes: - update pinned windows environment for testing purposes
for more information, see https://pre-commit.ci
…ctional_lossy_links into dag_fix
for more information, see https://pre-commit.ci
This pull request addresses issue #1307.
Since it updates Snakemake to version 8, this fully breaks the Snakefile and is thus currently not operational. Additionally, switching to a newer Snakemake version requires a newer Python version as well, which is incompatible with the current matplotlib specifications.
Thus, this PR requires further investigation and more changes.
Closes #1307 .
Changes proposed in this Pull Request
Major changes:
Minor changes:
Checklist
envs/environment.yaml
anddoc/requirements.txt
.config.default.yaml
andconfig.tutorial.yaml
.test/
(note tests are changing the config.tutorial.yaml)doc/configtables/*.csv
and line references are adjusted indoc/configuration.rst
anddoc/tutorial.rst
.doc/release_notes.rst
is amended in the format of previous release notes, including reference to the requested PR.