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

Do not mute mutable arguments #251

Open
sebastian-schindler opened this issue Jan 30, 2024 · 1 comment
Open

Do not mute mutable arguments #251

sebastian-schindler opened this issue Jan 30, 2024 · 1 comment

Comments

@sebastian-schindler
Copy link

The current implementation of corner_impl is muting its mutable arguments, like here:

if hist_kwargs is None:
    hist_kwargs = dict()
hist_kwargs["color"] = hist_kwargs.get("color", color)

The dictionary hist_kwargs as seen from the scope calling corner is changed after the call.

Minimal example:

import numpy as np
import corner
mydict = {}
corner.corner(np.array([[1,2], [3,4]]), hist_kwargs=mydict)
print(mydict)

The object mydict was changed by the call to corner, which is unexpected.

This could be resolved by simply adding hist_kwargs = dict(hist_kwargs), because then the rest of the code is working on a copy of the dictionary. Of course the user could do this as well, but it would be nicer if the user wouldn't need to bother. Also, this becomes more annoying when passing this argument as a kwarg itself, because then a copy.deepcopy becomes necessary to ensure that what is passed in stays the same.

(If this will be implemented: If the default for hist_kwargs would not be None, but [], the if checking for None wouldn't also be needed anymore, because dict([]) gives an empty dictionary.)

@dfm
Copy link
Owner

dfm commented Feb 7, 2024

Thanks for the report! I'd be happy to review a PR that adds the appropriate copy, but please keep the same default value since some downstream dependencies rely on that behavior.

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

No branches or pull requests

2 participants