Skip to content
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

Add an option to create a filegraph in the dag rule #1574

Merged
merged 11 commits into from
Mar 18, 2025

Conversation

tgilon
Copy link
Collaborator

@tgilon tgilon commented Mar 12, 2025

Closes # (if applicable).

Changes proposed in this Pull Request

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.

@tgilon tgilon marked this pull request as ready for review March 12, 2025 16:18
Copy link
Member

@fneum fneum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice utility! But avoid definition of dag_type outside rule definition.

It seems the current approach mixes two approaches (wildcards and parameters). With the current setup you could actually use

snakemake --{dag_type} all | sed -n "/digraph/,\$p" > {output.dot}

to run snakemake dag_filegraph.pdf or snakemake dag_rulegraph.pdf and forget about the params or config setting.

I would also be fine to just create both rulegraph and filegraph on run snakemake dag.

Snakefile Outdated
Comment on lines 114 to 116
dag_type = config.get("dag_type", "rulegraph")


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dag_type = config.get("dag_type", "rulegraph")

Snakefile Outdated
conda:
"envs/environment.yaml"
params:
dag_type=dag_type,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dag_type=dag_type,
dag_type=config.get("dag_type", "rulegraph"),

Co-authored-by: Fabian Neumann <fabian.neumann@outlook.de>
@tgilon
Copy link
Collaborator Author

tgilon commented Mar 14, 2025

Thanks @fneum for the review. Using only wildcards implies that we cannot have a default behaviour. That's why I suggest using config and a variable. I think the dag rule needs to be an easy to use rule as the dag is key to understanding the workflow (especially in the beginning).

# Not possible
snakemake dag

# Possible
snakemake resources/dag_rulegraph.pdf
snakemake resources/dag_filegraph.pdf

Also, as the filegraph may not be useful for everyone, I thought it would be a good idea to make it optional.

Would there be an implementation with wildcards and a default value?

@fneum
Copy link
Member

fneum commented Mar 17, 2025

Another option would be two separate rules:

snakemake rulegraph
snakemake filegraph

@fneum fneum merged commit a5b1489 into PyPSA:master Mar 18, 2025
2 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.

2 participants