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

fix: Use 'snakemake.utils.update_config' instead of 'dict.update' #1126

Merged
merged 23 commits into from Nov 25, 2021

Conversation

kpj
Copy link
Contributor

@kpj kpj commented Aug 9, 2021

Description

When merging configs, Snakemake uses dict.update instead of snakemake.utils.update_config which is not recursive.
Nested config options can thus disappear unexpectedly.

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

@kpj kpj marked this pull request as ready for review August 9, 2021 14:45
@sonarcloud
Copy link

sonarcloud bot commented Aug 12, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@kpj
Copy link
Contributor Author

kpj commented Aug 29, 2021

@johanneskoester Any thoughts on this? :-)

Copy link
Contributor

@johanneskoester johanneskoester left a comment

Choose a reason for hiding this comment

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

Makes sense to me! Sorry for the late review.

@sonarcloud
Copy link

sonarcloud bot commented Oct 21, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johanneskoester
Copy link
Contributor

Soemthing fails on windows now. Do you have a clue?

@kpj
Copy link
Contributor Author

kpj commented Oct 21, 2021

Not really, and I don't have any Windows machine to investigate this :-(
Maybe something related to the quotation marks?

@sonarcloud
Copy link

sonarcloud bot commented Nov 24, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johanneskoester johanneskoester merged commit 2658027 into snakemake:main Nov 25, 2021
@johanneskoester
Copy link
Contributor

Thanks a lot, finally done :-).

@kpj
Copy link
Contributor Author

kpj commented Nov 25, 2021

Wonderful! Thanks a lot for the debugging as well :-)

@joverlee521 joverlee521 mentioned this pull request Oct 27, 2023
2 tasks
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.

None yet

2 participants