Mutable default arguments and potential bugs in the future

[jparas-3]

There are instances where mutable default arguments (empty lists/dicts) are used in function/method definitions, such as in amptorch.trainer.Atoms.init, which has a config default value of an empty dict. You should never use mutable default arguments in function definitions, since every call to that function that uses the default argument will share that same list/dict, which can cause some weird bugs.

The safe alternative is to set the default value to None, check if the argument's value is None, and then assign it accordingly.

For the specific init example, I think a better alternative would just be to get rid of the default value entirely in order to enforce the config as an argument. Not sure where else this issue pops up, but it should be easy to find all instances of '=[]' and '={}' using grep. This is potentially an issue in other medford-group repos as well.

[ajmedford]

Hi Jacob,

Thanks for pointing this out. I don't expect that changing this would break anything, although @mshuaibii or @nicoleyghu can confirm. If you (or anyone else) has time to make the changes and submit a PR that would be great!

Here is an article detailing the issue for reference: https://docs.python-guide.org/writing/gotchas/

Best,
AJ