fix: replace mutable default arguments with None in CNV plot functions#884
Conversation
Fixes malariagen#787 plot_cnv_hmm_heatmap, plot_cnv_hmm_heatmap_track, and the CNV colorscale in plot_diplotype_clustering_advanced were using a mutable list (cnv_params.colorscale_default) as a default parameter value, which is a dangerous pattern in Python. Replaced with None and added fallback assignment at the start of each function body.
There was a problem hiding this comment.
Pull request overview
This PR removes mutable default argument values in CNV-related plotting APIs by switching the defaults to None and applying the existing cnv_params.colorscale_default inside the function body, addressing the “dangerous-default-value” warning in issue #787.
Changes:
- Updated
plot_diplotype_clustering_advanced()to defaultcnv_colorscaletoNoneand assign the CNV default colorscale at runtime. - Updated
plot_cnv_hmm_heatmap_track()andplot_cnv_hmm_heatmap()to defaultpalettetoNoneand assign the CNV default palette at runtime.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
malariagen_data/anoph/dipclust.py |
Replaces CNV colorscale mutable default arg with None + in-function fallback. |
malariagen_data/anoph/cnv_data.py |
Replaces HMM heatmap palette mutable default args with None + in-function fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you, @PaulaYaniz. Technically, the problem with mutable lists as parameters only exist if there is a possibility that one of the parameters is going to be modified during the execution. Almost none of the user-facing functions should ever modify their parameters, and certainly none of the plotting functions. It is harder to check, however, so I support your proposed changes. |
2 participants
Fixes #787
plot_cnv_hmm_heatmap, plot_cnv_hmm_heatmap_track, and the CNV colorscale in plot_diplotype_clustering_advanced were using a mutable list (cnv_params.colorscale_default) as a default parameter value, which is a dangerous pattern in Python. Replaced with None and added fallback assignment at the start of each function body.