Skip to content

remove config.yaml#1649

Merged
lkstrp merged 3 commits intoPyPSA:masterfrom
lkstrp:remove-config-yaml
May 26, 2025
Merged

remove config.yaml#1649
lkstrp merged 3 commits intoPyPSA:masterfrom
lkstrp:remove-config-yaml

Conversation

@lkstrp
Copy link
Copy Markdown
Member

@lkstrp lkstrp commented Apr 17, 2025

Closes # (if applicable).

Changes proposed in this Pull Request

  • Remove config/config.yaml from the snakemake config chain
    • Currently the chain is config/config.default.yaml, config/plotting.default.yaml, config/config.yaml and then your passed config. For PyPSA-DE another conifg is added
    • The function copy_default_files is very confusing and not good snakemake practice I would assume. The created config.yaml can lead to an artifact being used without the user realising it
    • If we need a template which is not the default config, we can add a config.template.yaml but not run it by default

Removing this is quite breaking. If we agree, I'll adjust the docs, release notes etc.

Checklist

  • I tested my contribution locally and it works as intended.
  • Code and workflow changes are sufficiently documented.
  • Changed dependencies are added to envs/environment.yaml.
  • Changes in configuration options are added in config/config.default.yaml.
  • Changes in configuration options are documented in doc/configtables/*.csv.
  • Sources of newly added data are documented in doc/data_sources.rst.
  • A release note doc/release_notes.rst is added.

@FabianHofmann
Copy link
Copy Markdown
Contributor

I agree with removing the full copy to defaults to config.yaml. this lead to a lot of issues. however removing the config.yaml completely, I am not sure about. it is quite breaking (which should be fine as this is our policy in pypsa-eur). just to get the rationale: The idea is that the user always sets a --configfile argument in the snakemake call as soon as they want to divert from the default?

@lkstrp
Copy link
Copy Markdown
Member Author

lkstrp commented Apr 17, 2025

So you would remove the complete default copy, but keep configfile: "config/config.yaml" in the snakefile? So if people want to avoid passing --configfile on every run, they can still manually create and use that file?

Sounds good to me. You would still have the fourfold chain if you pass your own config file, and it is not super obvious that a config.yaml that does not even exist in the repo is being read in, but probably worth it for the convenience. Also makes this change less breaking

@daniel-rdt
Copy link
Copy Markdown
Contributor

@lkstrp Is there any update on this?

@lkstrp
Copy link
Copy Markdown
Member Author

lkstrp commented May 20, 2025

No, we just need to choose one of the two options:

  1. Remove config.yaml completely; you would always need to add --configfile as a flag
  2. Remove config.yaml from the repository, but keep it in the chain so that it can still be used without adding a flag

I guess people would prefer the second

@daniel-rdt
Copy link
Copy Markdown
Contributor

daniel-rdt commented May 20, 2025

No, we just need to choose one of the two options:

  1. Remove config.yaml completely; you would always need to add --configfile as a flag
  2. Remove config.yaml from the repository, but keep it in the chain so that it can still be used without adding a flag

I guess people would prefer the second

I personally find that having config.yaml in the chain can be quite annoying and also confusing, especially, for first time users (because it often happened by accident that the config.yaml is in there as an outdated copy of the default config) which leads to problems.

I think we should remove the copy behaviour as @FabianHofmann already suggested, but we could keep the config.yaml in the chain but have it as an empty custom config file per default. This way by default it does not overwrite anything but only does so if the user explicitly adds anything inside this custom config? We could even rename the file per default to something like: config.custom.yaml or some similar naming.

tl;dr I agree with the upside of option two if the copy behaviour is removed

@fneum
Copy link
Copy Markdown
Member

fneum commented May 21, 2025

So you would remove the complete default copy, but keep configfile: "config/config.yaml" in the snakefile? So if people want to avoid passing --configfile on every run, they can still manually create and use that file?

This sounds like the best compromise. The important aspects to me are:

  • No requirement to pass --configfile
  • Untracked config.yaml file. (Otherwise, I bet most PRs will have contributors config)
  • No renaming so it's not breaking.
  • Agree to remove default copy.

@lkstrp
Copy link
Copy Markdown
Member Author

lkstrp commented May 26, 2025

This is good to go now

@lkstrp lkstrp merged commit 2c1daf4 into PyPSA:master May 26, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants